Skip to content

Commit

Permalink
RBAC: Block withRefreshCatalog in WebBackendConnectionGet unless work…
Browse files Browse the repository at this point in the history
…space editor or above (#11326)
  • Loading branch information
pmossman committed Feb 21, 2024
1 parent 863922f commit f194a0f
Show file tree
Hide file tree
Showing 4 changed files with 207 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
import static io.airbyte.commons.auth.AuthRoleConstants.WORKSPACE_EDITOR;
import static io.airbyte.commons.auth.AuthRoleConstants.WORKSPACE_READER;

import authorization.AirbyteApiAuthorizationHelper;
import authorization.Scope;
import io.airbyte.api.generated.WebBackendApi;
import io.airbyte.api.model.generated.ConnectionIdRequestBody;
import io.airbyte.api.model.generated.ConnectionStateType;
import io.airbyte.api.model.generated.PermissionType;
import io.airbyte.api.model.generated.WebBackendCheckUpdatesRead;
import io.airbyte.api.model.generated.WebBackendConnectionCreate;
import io.airbyte.api.model.generated.WebBackendConnectionListRequestBody;
Expand All @@ -23,6 +26,7 @@
import io.airbyte.api.model.generated.WebBackendGeographiesListResult;
import io.airbyte.api.model.generated.WebBackendWorkspaceState;
import io.airbyte.api.model.generated.WebBackendWorkspaceStateResult;
import io.airbyte.commons.lang.MoreBooleans;
import io.airbyte.commons.server.handlers.WebBackendCheckUpdatesHandler;
import io.airbyte.commons.server.handlers.WebBackendConnectionsHandler;
import io.airbyte.commons.server.handlers.WebBackendGeographiesHandler;
Expand All @@ -33,6 +37,7 @@
import io.micronaut.scheduling.annotation.ExecuteOn;
import io.micronaut.security.annotation.Secured;
import io.micronaut.security.rules.SecurityRule;
import java.util.Set;

@Controller("/api/v1/web_backend")
@Secured(SecurityRule.IS_AUTHENTICATED)
Expand All @@ -41,13 +46,16 @@ public class WebBackendApiController implements WebBackendApi {
private final WebBackendConnectionsHandler webBackendConnectionsHandler;
private final WebBackendGeographiesHandler webBackendGeographiesHandler;
private final WebBackendCheckUpdatesHandler webBackendCheckUpdatesHandler;
private final AirbyteApiAuthorizationHelper airbyteApiAuthorizationHelper;

public WebBackendApiController(final WebBackendConnectionsHandler webBackendConnectionsHandler,
final WebBackendGeographiesHandler webBackendGeographiesHandler,
final WebBackendCheckUpdatesHandler webBackendCheckUpdatesHandler) {
final WebBackendCheckUpdatesHandler webBackendCheckUpdatesHandler,
final AirbyteApiAuthorizationHelper airbyteApiAuthorizationHelper) {
this.webBackendConnectionsHandler = webBackendConnectionsHandler;
this.webBackendGeographiesHandler = webBackendGeographiesHandler;
this.webBackendCheckUpdatesHandler = webBackendCheckUpdatesHandler;
this.airbyteApiAuthorizationHelper = airbyteApiAuthorizationHelper;
}

@Post("/state/get_type")
Expand Down Expand Up @@ -87,6 +95,14 @@ public WebBackendConnectionRead webBackendCreateConnection(final WebBackendConne
public WebBackendConnectionRead webBackendGetConnection(final WebBackendConnectionRequestBody webBackendConnectionRequestBody) {
return ApiHelper.execute(() -> {
TracingHelper.addConnection(webBackendConnectionRequestBody.getConnectionId());
if (MoreBooleans.isTruthy(webBackendConnectionRequestBody.getWithRefreshedCatalog())) {
// only allow refresh catalog if the user is at least a workspace editor or
// organization editor for the connection's workspace
airbyteApiAuthorizationHelper.checkWorkspacePermissions(
webBackendConnectionRequestBody.getConnectionId().toString(),
Scope.CONNECTION,
Set.of(PermissionType.WORKSPACE_EDITOR, PermissionType.ORGANIZATION_EDITOR));
}
return webBackendConnectionsHandler.webBackendGetConnection(webBackendConnectionRequestBody);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.airbyte.commons.server.support.AuthenticationHttpHeaders.DESTINATION_I
import io.airbyte.commons.server.support.AuthenticationHttpHeaders.JOB_ID_HEADER
import io.airbyte.commons.server.support.AuthenticationHttpHeaders.SOURCE_ID_HEADER
import io.airbyte.commons.server.support.AuthenticationHttpHeaders.WORKSPACE_IDS_HEADER
import io.airbyte.commons.server.support.CurrentUserService
import io.github.oshai.kotlinlogging.KotlinLogging
import jakarta.inject.Singleton
import java.util.UUID
Expand All @@ -26,6 +27,7 @@ private val logger = KotlinLogging.logger {}
class AirbyteApiAuthorizationHelper(
private val authorizationHeaderResolver: AuthenticationHeaderResolver,
private val permissionHandler: PermissionHandler,
private val currentUserService: CurrentUserService,
) {
private fun resolveIdsToWorkspaceIds(
ids: List<String>,
Expand Down Expand Up @@ -56,12 +58,30 @@ class AirbyteApiAuthorizationHelper(
}

/**
* Given a list of resolved workspace Ids and a userInfo header, confirm user access by checking against the list of
* workspace permissions for the user.
* Given a scoped ID, confirm that the current user has the given permission type.
*
* @param id - The ID we are checking permissions for
* @param scope - The scope of the ID
* @param permissionTypes - the set of permissions needed to access the resource(s).
* If the user has any of the permissions, the check will pass.
*
* @throws ForbiddenProblem - If the user does not have the required permissions
*/
fun checkWorkspacePermissions(
id: String,
scope: Scope,
permissionTypes: Set<PermissionType>,
) {
checkWorkspacePermissions(listOf(id), scope, currentUserService.currentUser.userId, permissionTypes)
}

/**
* Given a list of scoped IDs and a user ID, confirm that the indicated user
* has the given permission type.
*
* @param ids - The Ids we are checking permissions for
* @param scope - The scope of the Ids
* @param userInfo - The userInfo header
* @param userId - The ID of the user we are checking permissions for
* @param permissionType - the permission needed to access the resource(s)
*
* @throws ForbiddenProblem - If the user does not have the required permissions
Expand All @@ -71,6 +91,46 @@ class AirbyteApiAuthorizationHelper(
scope: Scope,
userId: UUID,
permissionType: PermissionType,
) {
checkWorkspacePermissions(ids, scope, userId, setOf(permissionType))
}

/**
* Given a list of scoped IDs, confirm that the current user has the
* given workspace permission type.
*
* @param ids - The Ids we are checking permissions for
* @param scope - The scope of the Ids
* @param permissionTypes - the set of permissions needed to access the resource(s).
* If the user has any of the permissions, the check will pass.
*
* @throws ForbiddenProblem - If the user does not have the required permissions
*/
fun checkWorkspacePermissions(
ids: List<String>,
scope: Scope,
permissionTypes: Set<PermissionType>,
) {
checkWorkspacePermissions(ids, scope, currentUserService.currentUser.userId, permissionTypes)
}

/**
* Given a list of scoped IDs, confirm that the current user has the
* given workspace permission type.
*
* @param ids - The Ids we are checking permissions for
* @param scope - The scope of the Ids
* @param userId - The ID of the user we are checking permissions for
* @param permissionTypes - the set of permissions needed to access the resource(s).
* If the user has any of the permissions, the check will pass.
*
* @throws ForbiddenProblem - If the user does not have the required permissions
*/
fun checkWorkspacePermissions(
ids: List<String>,
scope: Scope,
userId: UUID,
permissionTypes: Set<PermissionType>,
) {
logger.debug { "Checking workspace permissions for $ids in scope [${scope.name}]." }
if (ids.isEmpty() && scope != Scope.WORKSPACES) {
Expand All @@ -93,18 +153,31 @@ class AirbyteApiAuthorizationHelper(
throw ForbiddenProblem("Unable to resolve to a workspace for $ids in scope [${scope.name}].")
}

val request = PermissionsCheckMultipleWorkspacesRequest()
request.permissionType = permissionType
request.userId = userId
request.workspaceIds = resolvedWorkspaceIds

val permissionCheckRead = permissionHandler.permissionsCheckMultipleWorkspaces(request)

if (permissionCheckRead.status != PermissionCheckRead.StatusEnum.SUCCEEDED) {
if (!checkIfAnyPermissionGranted(resolvedWorkspaceIds, userId, permissionTypes)) {
throw ForbiddenProblem("User does not have the required permissions to access the resource(s) $ids of type [${scope.name}].")
}
}

private fun checkIfAnyPermissionGranted(
resolvedWorkspaceIds: List<UUID>,
userId: UUID,
permissionTypes: Set<PermissionType>,
): Boolean {
for (permissionType in permissionTypes) {
val request = PermissionsCheckMultipleWorkspacesRequest()
request.permissionType = permissionType
request.userId = userId
request.workspaceIds = resolvedWorkspaceIds

val permissionCheckRead = permissionHandler.permissionsCheckMultipleWorkspaces(request)

if (permissionCheckRead.status == PermissionCheckRead.StatusEnum.SUCCEEDED) {
return true
}
}
return false
}

private fun buildPropertiesMapForConnection(id: String): Map<String, String> {
return mapOf(Scope.CONNECTION.mappedHeaderProperty to id)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,30 @@

package io.airbyte.server.apis;

import authorization.AirbyteApiAuthorizationHelper;
import io.airbyte.api.model.generated.ConnectionStateType;
import io.airbyte.api.model.generated.SourceDefinitionIdRequestBody;
import io.airbyte.api.model.generated.SourceIdRequestBody;
import io.airbyte.api.model.generated.WebBackendCheckUpdatesRead;
import io.airbyte.api.model.generated.WebBackendConnectionRead;
import io.airbyte.api.model.generated.WebBackendConnectionReadList;
import io.airbyte.api.model.generated.WebBackendConnectionRequestBody;
import io.airbyte.api.model.generated.WebBackendGeographiesListResult;
import io.airbyte.api.model.generated.WebBackendWorkspaceStateResult;
import io.airbyte.api.server.problems.ForbiddenProblem;
import io.airbyte.commons.json.Jsons;
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.validation.json.JsonValidationException;
import io.micronaut.context.annotation.Primary;
import io.micronaut.context.annotation.Requires;
import io.micronaut.context.env.Environment;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpStatus;
import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
import jakarta.inject.Singleton;
import java.io.IOException;
import java.util.UUID;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

Expand All @@ -29,6 +36,23 @@
@SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")
class WebBackendApiTest extends BaseControllerTest {

private AirbyteApiAuthorizationHelper airbyteApiAuthorizationHelper;

// Due to some strange interaction between Micronaut 3, Java, and Kotlin, the only way to
// mock this Kotlin dependency is to annotate it with @Bean instead of @MockBean, and to
// declare it here instead of within the BaseControllerTest. May be able to move it
// back to BaseControllerTest and use @MockBean after we upgrade to Micronaut 4.
@Singleton
@Primary
AirbyteApiAuthorizationHelper mmAirbyteApiAuthorizationHelper() {
return airbyteApiAuthorizationHelper;
}

@BeforeEach
void setup() {
airbyteApiAuthorizationHelper = Mockito.mock(AirbyteApiAuthorizationHelper.class);
}

@Test
void testGetStateType() throws IOException {
Mockito.when(webBackendConnectionsHandler.getStateType(Mockito.any()))
Expand Down Expand Up @@ -65,16 +89,41 @@ void testWebBackendCreateConnection() throws JsonValidationException, ConfigNotF

@Test
void testWebBackendGetConnection() throws JsonValidationException, ConfigNotFoundException, IOException {
Mockito.when(webBackendConnectionsHandler.webBackendGetConnection(Mockito.any()))
.thenReturn(new WebBackendConnectionRead())
.thenThrow(new ConfigNotFoundException("", ""));
final String path = "/api/v1/web_backend/connections/get";

Mockito.when(webBackendConnectionsHandler.webBackendGetConnection(Mockito.any()))
.thenReturn(new WebBackendConnectionRead()) // first call that makes it here succeeds
.thenReturn(new WebBackendConnectionRead()) // second call that makes it here succeeds
.thenThrow(new ConfigNotFoundException("", "")); // third call that makes it here 404s

Mockito
.doNothing() // first call that makes it here passes auth check
.doNothing() // second call that makes it here passes auth check but 404s
.doThrow(new ForbiddenProblem("forbidden")) // third call fails auth check and 403s
.when(airbyteApiAuthorizationHelper).checkWorkspacePermissions(Mockito.anyString(), Mockito.any(), Mockito.any());

// first call doesn't activate checkWorkspacePermissions because withRefreshedCatalog is false
testEndpointStatus(
HttpRequest.POST(path, Jsons.serialize(new SourceIdRequestBody())),
HttpRequest.POST(path, Jsons.serialize(new WebBackendConnectionRequestBody().connectionId(UUID.randomUUID()).withRefreshedCatalog(false))),
HttpStatus.OK);

// second call activates checkWorkspacePermissions because withRefreshedCatalog is true, and passes
// the check
testEndpointStatus(
HttpRequest.POST(path, Jsons.serialize(new WebBackendConnectionRequestBody().connectionId(UUID.randomUUID()).withRefreshedCatalog(true))),
HttpStatus.OK);

// third call activates checkWorkspacePermissions because withRefreshedCatalog is true, passes it,
// but then fails on the 404
testErrorEndpointStatus(
HttpRequest.POST(path, Jsons.serialize(new SourceDefinitionIdRequestBody())),
HttpRequest.POST(path, Jsons.serialize(new WebBackendConnectionRequestBody().connectionId(UUID.randomUUID()).withRefreshedCatalog(true))),
HttpStatus.NOT_FOUND);

// fourth call activates checkWorkspacePermissions because withRefreshedCatalog is true, but fails
// the check, so 403s
testErrorEndpointStatus(
HttpRequest.POST(path, Jsons.serialize(new WebBackendConnectionRequestBody().connectionId(UUID.randomUUID()).withRefreshedCatalog(true))),
HttpStatus.FORBIDDEN);
}

@Test
Expand Down
Loading

0 comments on commit f194a0f

Please sign in to comment.