This is an automated email from the ASF dual-hosted git repository.
tpalfy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new 5aaf365468 NIFI-14514 ListSmb skips folders it cannot access due to
insufficient permissions
5aaf365468 is described below
commit 5aaf365468c661ee7e525f1b7a80d93e97f811e5
Author: Peter Turcsanyi <[email protected]>
AuthorDate: Tue May 6 18:19:51 2025 +0200
NIFI-14514 ListSmb skips folders it cannot access due to insufficient
permissions
This closes #9917.
Signed-off-by: Tamas Palfy <[email protected]>
---
.../services/smb/SmbClientProviderService.java | 3 +-
.../org/apache/nifi/processors/smb/FetchSmb.java | 4 +--
.../org/apache/nifi/processors/smb/ListSmb.java | 2 +-
.../apache/nifi/processors/smb/FetchSmbTest.java | 3 +-
.../apache/nifi/processors/smb/ListSmbTest.java | 5 +--
.../services/smb/SmbjClientProviderService.java | 5 +--
.../nifi/services/smb/SmbjClientService.java | 22 ++++++++++--
.../nifi/services/smb/SmbjClientServiceIT.java | 40 +++++++++++++++++++++-
.../nifi/services/smb/SmbjClientServiceTest.java | 39 ++++++++++++++-------
9 files changed, 99 insertions(+), 24 deletions(-)
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-client-api/src/main/java/org/apache/nifi/services/smb/SmbClientProviderService.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-client-api/src/main/java/org/apache/nifi/services/smb/SmbClientProviderService.java
index 3dc23f1aa0..c451d9e45c 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-client-api/src/main/java/org/apache/nifi/services/smb/SmbClientProviderService.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-client-api/src/main/java/org/apache/nifi/services/smb/SmbClientProviderService.java
@@ -19,6 +19,7 @@ package org.apache.nifi.services.smb;
import java.io.IOException;
import java.net.URI;
import org.apache.nifi.controller.ControllerService;
+import org.apache.nifi.logging.ComponentLog;
public interface SmbClientProviderService extends ControllerService {
@@ -34,6 +35,6 @@ public interface SmbClientProviderService extends
ControllerService {
*
* @return the client.
*/
- SmbClientService getClient() throws IOException;
+ SmbClientService getClient(ComponentLog logger) throws IOException;
}
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/FetchSmb.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/FetchSmb.java
index 52feec30f1..92d1a4d5ad 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/FetchSmb.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/FetchSmb.java
@@ -150,7 +150,7 @@ public class FetchSmb extends AbstractProcessor {
final SmbClientProviderService clientProviderService =
context.getProperty(SMB_CLIENT_PROVIDER_SERVICE).asControllerService(SmbClientProviderService.class);
- try (SmbClientService client = clientProviderService.getClient()) {
+ try (SmbClientService client =
clientProviderService.getClient(getLogger())) {
flowFile = session.write(flowFile, outputStream ->
client.readFile(filePath, outputStream));
session.transfer(flowFile, REL_SUCCESS);
@@ -183,7 +183,7 @@ public class FetchSmb extends AbstractProcessor {
final SmbClientProviderService clientProviderService =
context.getProperty(SMB_CLIENT_PROVIDER_SERVICE).asControllerService(SmbClientProviderService.class);
- try (SmbClientService client = clientProviderService.getClient()) {
+ try (SmbClientService client =
clientProviderService.getClient(getLogger())) {
if (completionStrategy == CompletionStrategy.MOVE) {
final String destinationDirectory =
context.getProperty(DESTINATION_DIRECTORY).evaluateAttributeExpressions(attributes).getValue();
final boolean createDestinationDirectory =
context.getProperty(CREATE_DESTINATION_DIRECTORY).asBoolean();
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/ListSmb.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/ListSmb.java
index b2f45ec4a5..156659e1d5 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/ListSmb.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/main/java/org/apache/nifi/processors/smb/ListSmb.java
@@ -443,7 +443,7 @@ public class ListSmb extends
AbstractListProcessor<SmbListableEntity> {
final SmbClientProviderService clientProviderService =
context.getProperty(SMB_CLIENT_PROVIDER_SERVICE).asControllerService(SmbClientProviderService.class);
final String directory = getDirectory(context);
- final SmbClientService clientService =
clientProviderService.getClient();
+ final SmbClientService clientService =
clientProviderService.getClient(getLogger());
return clientService.listFiles(directory).onClose(() -> {
try {
clientService.close();
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/FetchSmbTest.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/FetchSmbTest.java
index 6847fccb41..1b344a1bd5 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/FetchSmbTest.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/FetchSmbTest.java
@@ -23,6 +23,7 @@ import java.net.URI;
import java.util.HashMap;
import java.util.Map;
import org.apache.commons.io.IOUtils;
+import org.apache.nifi.logging.ComponentLog;
import org.apache.nifi.services.smb.SmbClientProviderService;
import org.apache.nifi.services.smb.SmbClientService;
import org.apache.nifi.services.smb.SmbException;
@@ -62,7 +63,7 @@ class FetchSmbTest {
@BeforeEach
public void beforeEach() throws Exception {
mockCloseable = MockitoAnnotations.openMocks(this);
-
when(clientProviderService.getClient()).thenReturn(mockNifiSmbClientService);
+
when(clientProviderService.getClient(any(ComponentLog.class))).thenReturn(mockNifiSmbClientService);
when(clientProviderService.getIdentifier()).thenReturn(CLIENT_SERVICE_PROVIDER_ID);
when(clientProviderService.getServiceLocation()).thenReturn(URI.create("smb://localhost:445/share"));
}
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/ListSmbTest.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/ListSmbTest.java
index cacf587634..2c13fbdb17 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/ListSmbTest.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-processors/src/test/java/org/apache/nifi/processors/smb/ListSmbTest.java
@@ -48,6 +48,7 @@ import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.nifi.distributed.cache.client.DistributedMapCacheClient;
+import org.apache.nifi.logging.ComponentLog;
import org.apache.nifi.processor.util.list.ListedEntity;
import org.apache.nifi.processors.smb.util.InitialListingStrategy;
import org.apache.nifi.services.smb.SmbClientProviderService;
@@ -113,7 +114,7 @@ class ListSmbTest {
final SmbClientProviderService clientProviderService =
mock(SmbClientProviderService.class);
when(clientProviderService.getIdentifier()).thenReturn("different-client-provider");
when(clientProviderService.getServiceLocation()).thenReturn(URI.create("smb://localhost:445/share"));
-
when(clientProviderService.getClient()).thenReturn(mockNifiSmbClientService);
+
when(clientProviderService.getClient(any(ComponentLog.class))).thenReturn(mockNifiSmbClientService);
testRunner.setProperty(SMB_CLIENT_PROVIDER_SERVICE,
"different-client-provider");
testRunner.addControllerService("different-client-provider",
clientProviderService);
testRunner.enableControllerService(clientProviderService);
@@ -309,7 +310,7 @@ class ListSmbTest {
testRunner.setProperty(DIRECTORY, "testDirectory");
final SmbClientProviderService clientProviderService =
mockSmbClientProviderService();
-
when(clientProviderService.getClient()).thenReturn(mockNifiSmbClientService);
+
when(clientProviderService.getClient(any(ComponentLog.class))).thenReturn(mockNifiSmbClientService);
testRunner.setProperty(SMB_CLIENT_PROVIDER_SERVICE,
CLIENT_SERVICE_PROVIDER_ID);
testRunner.addControllerService(CLIENT_SERVICE_PROVIDER_ID,
clientProviderService);
testRunner.enableControllerService(clientProviderService);
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java
index 7d6258c1ee..85c773daff 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java
@@ -29,6 +29,7 @@ import org.apache.nifi.annotation.lifecycle.OnEnabled;
import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.controller.AbstractControllerService;
import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.logging.ComponentLog;
import java.io.IOException;
import java.net.URI;
@@ -149,7 +150,7 @@ public class SmbjClientProviderService extends
AbstractControllerService impleme
}
@Override
- public SmbClientService getClient() throws IOException {
+ public SmbClientService getClient(final ComponentLog logger) throws
IOException {
final Connection connection = smbClient.connect(hostname, port);
final Session session;
@@ -173,7 +174,7 @@ public class SmbjClientProviderService extends
AbstractControllerService impleme
throw new IllegalArgumentException("DiskShare not found. Share " +
share.getClass().getSimpleName() + " found on " + getServiceLocation());
}
- return new SmbjClientService(session, (DiskShare) share,
getServiceLocation());
+ return new SmbjClientService(session, (DiskShare) share,
getServiceLocation(), logger);
}
private void closeSession(final Session session) {
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java
index 4e04bf85d1..6e0ab503b6 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java
@@ -31,6 +31,7 @@ import com.hierynomus.smbj.session.Session;
import com.hierynomus.smbj.share.Directory;
import com.hierynomus.smbj.share.DiskShare;
import com.hierynomus.smbj.share.File;
+import org.apache.nifi.logging.ComponentLog;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -51,11 +52,13 @@ class SmbjClientService implements SmbClientService {
private final Session session;
private final DiskShare share;
private final URI serviceLocation;
+ private final ComponentLog logger;
- SmbjClientService(final Session session, final DiskShare share, final URI
serviceLocation) {
+ SmbjClientService(final Session session, final DiskShare share, final URI
serviceLocation, final ComponentLog logger) {
this.session = session;
this.share = share;
this.serviceLocation = serviceLocation;
+ this.logger = logger;
}
@Override
@@ -72,7 +75,22 @@ class SmbjClientService implements SmbClientService {
@Override
public Stream<SmbListableEntity> listFiles(final String directoryPath) {
return Stream.of(directoryPath).flatMap(path -> {
- final Directory directory = openDirectory(path);
+ final Directory directory;
+ try {
+ directory = openDirectory(path);
+ } catch (SMBApiException e) {
+ if (e.getStatus() == NtStatus.STATUS_ACCESS_DENIED) {
+ logger.warn("Could not list directory [{}/{}] because the
user does not have access to it.", serviceLocation, path, e);
+ return Stream.empty();
+ } else if (e.getStatus() == NtStatus.STATUS_BAD_NETWORK_NAME) {
+ // DFS resolution may return STATUS_BAD_NETWORK_NAME if
the user does not have access at share level
+ logger.warn("Could not list directory [{}/{}] because the
share does not exist or the user does not have access to it.", serviceLocation,
path, e);
+ return Stream.empty();
+ } else {
+ throw e;
+ }
+ }
+
return stream(directory::spliterator, 0, false)
.map(entity -> buildSmbListableEntity(entity, path,
serviceLocation))
.filter(entity -> !specialDirectory(entity))
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceIT.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceIT.java
index fc1d4ca3b5..5d34305b4a 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceIT.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceIT.java
@@ -24,8 +24,10 @@ import static
org.apache.nifi.services.smb.SmbjClientProviderService.PORT;
import static org.apache.nifi.services.smb.SmbjClientProviderService.SHARE;
import static org.apache.nifi.services.smb.SmbjClientProviderService.USERNAME;
import static org.apache.nifi.smb.common.SmbProperties.TIMEOUT;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
import eu.rekawek.toxiproxy.Proxy;
import eu.rekawek.toxiproxy.ToxiproxyClient;
@@ -41,6 +43,7 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.logging.ComponentLog;
import org.apache.nifi.util.MockConfigurationContext;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -130,7 +133,7 @@ public class SmbjClientServiceIT {
SmbClientService s = null;
try {
- s = smbjClientProviderService.getClient();
+ s =
smbjClientProviderService.getClient(mock(ComponentLog.class));
if (iteration == 25) {
proxy.toxics().bandwidth("CUT_CONNECTION_DOWNSTREAM",
ToxicDirection.DOWNSTREAM, 0L);
proxy.toxics().bandwidth("CUT_CONNECTION_UPSTREAM",
ToxicDirection.UPSTREAM, 0L);
@@ -176,6 +179,41 @@ public class SmbjClientServiceIT {
smbjClientProviderService.onDisabled();
}
+ @Test
+ public void shouldContinueListingAfterPermissionDenied() throws Exception {
+ writeFile("testDirectory/directory1/file1", "content");
+ writeFile("testDirectory/directory2/file2", "content");
+ writeFile("testDirectory/directory3/file3", "content");
+
+ sambaContainer.execInContainer("bash", "-c", "chmod 000
/folder/testDirectory/directory2");
+
+ SmbjClientProviderService smbjClientProviderService = new
SmbjClientProviderService();
+
+ Map<PropertyDescriptor, String> properties = new HashMap<>();
+ properties.put(HOSTNAME, sambaContainer.getHost());
+ properties.put(PORT,
String.valueOf(sambaContainer.getMappedPort(445)));
+ properties.put(SHARE, "share");
+ properties.put(USERNAME, "username");
+ properties.put(PASSWORD, "password");
+ properties.put(DOMAIN, "domain");
+ properties.put(TIMEOUT, "0.5 sec");
+
+ MockConfigurationContext mockConfigurationContext = new
MockConfigurationContext(properties, null, null);
+ smbjClientProviderService.onEnabled(mockConfigurationContext);
+
+ SmbClientService smbClientService =
smbjClientProviderService.getClient(mock(ComponentLog.class));
+
+ final Set<String> actual = smbClientService.listFiles("testDirectory")
+ .map(SmbListableEntity::getIdentifier)
+ .collect(toSet());
+
+ assertEquals(2, actual.size());
+ assertTrue(actual.contains("testDirectory/directory1/file1"));
+ assertTrue(actual.contains("testDirectory/directory3/file3"));
+
+ smbjClientProviderService.onDisabled();
+ }
+
private void writeFile(String path, String content) {
String containerPath = "/folder/" + path;
sambaContainer.copyFileToContainer(Transferable.of(content),
containerPath);
diff --git
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceTest.java
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceTest.java
index 13fafdf58b..fdaf8ab714 100644
---
a/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceTest.java
+++
b/nifi-extension-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/test/java/org/apache/nifi/services/smb/SmbjClientServiceTest.java
@@ -16,16 +16,22 @@
*/
package org.apache.nifi.services.smb;
+import com.hierynomus.mserref.NtStatus;
+import com.hierynomus.mssmb2.SMBApiException;
import com.hierynomus.smbj.session.Session;
import com.hierynomus.smbj.share.DiskShare;
-import org.junit.jupiter.api.AfterEach;
+import org.apache.nifi.logging.ComponentLog;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -37,26 +43,21 @@ class SmbjClientServiceTest {
@Mock
DiskShare share;
+ @Mock
+ ComponentLog logger;
+
@InjectMocks
SmbjClientService underTest;
- private AutoCloseable mockCloseable;
-
@BeforeEach
- public void beforeEach() {
+ void beforeEach() {
MockitoAnnotations.openMocks(this);
- }
- @AfterEach
- public void closeMock() throws Exception {
- if (mockCloseable != null) {
- mockCloseable.close();
- }
+ when(session.connectShare(anyString())).thenReturn(share);
}
@Test
- public void shouldCreateDirectoriesRecursively() throws Exception {
- when(session.connectShare(anyString())).thenReturn(share);
+ void ensureDirectoryShouldCreateDirectoriesRecursively() throws Exception {
when(share.fileExists("directory")).thenReturn(true);
when(share.fileExists("path")).thenReturn(false);
when(share.fileExists("to")).thenReturn(false);
@@ -67,7 +68,21 @@ class SmbjClientServiceTest {
verify(share).mkdir("directory/path");
verify(share).mkdir("directory/path/to");
verify(share).mkdir("directory/path/to/create");
+ }
+ @Test
+ void listFilesShouldHandlePermissionErrors() {
+ mockOpenDirectory("dir1", NtStatus.STATUS_ACCESS_DENIED);
+ mockOpenDirectory("dir2", NtStatus.STATUS_BAD_NETWORK_NAME);
+ mockOpenDirectory("dir3", NtStatus.STATUS_OTHER);
+
+ assertEquals(0, underTest.listFiles("dir1").count());
+ assertEquals(0, underTest.listFiles("dir2").count());
+ assertThrows(SMBApiException.class, () ->
underTest.listFiles("dir3").count());
}
+ private void mockOpenDirectory(String directoryName, NtStatus
responseStatus) {
+ when(share.openDirectory(eq(directoryName), any(), any(), any(),
any(), any()))
+ .thenThrow(new SMBApiException(responseStatus.getValue(),
null, null));
+ }
}
\ No newline at end of file
