Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -7,6 +7,7 @@
import com.sap.cds.feature.attachments.oss.client.OSClientFactory;
import com.sap.cds.feature.attachments.oss.handler.OSSAttachmentsServiceHandler;
import com.sap.cds.feature.attachments.oss.handler.TenantCleanupHandler;
import com.sap.cds.services.ServiceException;
import com.sap.cds.services.environment.CdsEnvironment;
import com.sap.cds.services.runtime.CdsRuntimeConfiguration;
import com.sap.cds.services.runtime.CdsRuntimeConfigurer;
Expand All @@ -30,6 +31,17 @@ public void eventHandlers(CdsRuntimeConfigurer configurer) {
boolean multitenancyEnabled = isMultitenancyEnabled(env);
String objectStoreKind = getObjectStoreKind(env);

// Fail fast on an insecure multitenant configuration: without a shared object store the
// object keys are not prefixed with the tenant ID, so tenants are not isolated in the
// bucket. Refuse to start rather than silently storing all tenants under the same keyspace.
if (multitenancyEnabled && !"shared".equals(objectStoreKind)) {
throw new ServiceException(
"Multitenancy is enabled but the object store kind is not 'shared' (was: '{}'). "
+ "A shared object store is required to isolate tenants via tenant-prefixed object "
+ "keys. Configure 'cds.attachments.objectStore.kind: shared'.",
objectStoreKind);
Comment thread
Schmarvinius marked this conversation as resolved.
}

// Fixed thread pool for background I/O operations (upload, download, delete).
// Default 16 is tuned for I/O-bound cloud storage calls, not CPU-bound work.
int threadPoolSize =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
package com.sap.cds.feature.attachments.oss.configuration;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand All @@ -11,6 +12,7 @@
import static org.mockito.Mockito.when;

import com.sap.cds.feature.attachments.oss.handler.OSSAttachmentsServiceHandler;
import com.sap.cds.services.ServiceException;
import com.sap.cds.services.environment.CdsEnvironment;
import com.sap.cds.services.runtime.CdsRuntime;
import com.sap.cds.services.runtime.CdsRuntimeConfigurer;
Expand Down Expand Up @@ -87,29 +89,31 @@ void testEventHandlersNoBindingDoesNotRegister() {
}

@Test
void testMtEnabledNonSharedKindRegistersOnlyOSSHandler() {
void testMtEnabledNonSharedKindFailsStartup() {
when(environment.getServiceBindings()).thenReturn(Stream.of(awsBinding));
when(environment.getProperty("cds.multitenancy.enabled", Boolean.class, Boolean.FALSE))
.thenReturn(Boolean.TRUE);
when(environment.getProperty("cds.attachments.objectStore.kind", String.class, null))
.thenReturn("dedicated");

registration.eventHandlers(configurer);
assertThatThrownBy(() -> registration.eventHandlers(configurer))
.isInstanceOf(ServiceException.class)
.hasMessageContaining("shared");

verify(configurer, times(1)).eventHandler(any(OSSAttachmentsServiceHandler.class));
verify(configurer, times(1)).eventHandler(any());
verify(configurer, never()).eventHandler(any());
}

@Test
void testMtEnabledNullKindRegistersOnlyOSSHandler() {
void testMtEnabledNullKindFailsStartup() {
when(environment.getServiceBindings()).thenReturn(Stream.of(awsBinding));
when(environment.getProperty("cds.multitenancy.enabled", Boolean.class, Boolean.FALSE))
.thenReturn(Boolean.TRUE);
Comment thread
Schmarvinius marked this conversation as resolved.

registration.eventHandlers(configurer);
assertThatThrownBy(() -> registration.eventHandlers(configurer))
.isInstanceOf(ServiceException.class)
.hasMessageContaining("shared");

verify(configurer, times(1)).eventHandler(any(OSSAttachmentsServiceHandler.class));
verify(configurer, times(1)).eventHandler(any());
verify(configurer, never()).eventHandler(any());
}

@Test
Expand Down
Loading