fix(snl): repair the SFTP repository and drop the sftpretty wrapper

pyproject.toml

@@ -73,7 +73,7 @@ dependencies = [
     "netaddr>=0.9.0",
     "wand>=0.6.13",
     "webdavclient3>=3.14.6",
-    "sftpretty>=1.0.0",
+    "paramiko>=5.0.0",
     "python-slugify",
     "orcid>=1.0.3",
     "python3-saml>=1.16.0",

sonar/config_sonar.py

@@ -544,6 +544,9 @@
 SONAR_APP_FTP_SNL_USER = "changeMe"
 SONAR_APP_FTP_SNL_PASSWORD = "changeMe"
 SONAR_APP_FTP_SNL_PATH = "changeMe"
+# Public host key of the SNL server, as a `known_hosts` line. When it is not
+# set, the host key is looked up in the `known_hosts` file of the user.
+SONAR_APP_FTP_SNL_HOST_KEY = ""
 SONAR_APP_SNL_EMAIL_TEMPLATE = "sonar/modules/documents/templates/documents/emailSNL.txt"
 
 """FTP connection to SNL server."""

sonar/modules/documents/cli/urn.py

@@ -82,6 +82,20 @@ def register():
     click.secho(f"{idx} URN registered.", fg="green")
 
 
+def create_snl_repository():
+    """Create a SNL repository from the application configuration.
+
+    :returns: SNLRepository object.
+    """
+    return SNLRepository(
+        host=current_app.config.get("SONAR_APP_FTP_SNL_HOST"),
+        user=current_app.config.get("SONAR_APP_FTP_SNL_USER"),
+        password=current_app.config.get("SONAR_APP_FTP_SNL_PASSWORD"),
+        directory=current_app.config.get("SONAR_APP_FTP_SNL_PATH"),
+        host_key=current_app.config.get("SONAR_APP_FTP_SNL_HOST_KEY"),
+    )
+
+
 @urn.command("snl-upload-file")
 @click.argument("urn_code")
 @with_appcontext
@@ -112,12 +126,7 @@ def snl_upload_file(urn_code):
         click.secho("Error: the document does not contains any files.")
         return
 
-    snl_repository = SNLRepository(
-        host=current_app.config.get("SONAR_APP_FTP_SNL_HOST"),
-        user=current_app.config.get("SONAR_APP_FTP_SNL_USER"),
-        password=current_app.config.get("SONAR_APP_FTP_SNL_PASSWORD"),
-        directory=current_app.config.get("SONAR_APP_FTP_SNL_PATH"),
-    )
+    snl_repository = create_snl_repository()
     snl_repository.connect()
 
     dnb_base_urn = current_app.config.get("SONAR_APP_FTP_SNL_PATH")
@@ -134,6 +143,8 @@ def snl_upload_file(urn_code):
         except Exception as exception:
             click.secho(str(exception), fg="red")
 
+    snl_repository.close()
+
     # print email template
     template_email_snl = current_app.config.get("SONAR_APP_SNL_EMAIL_TEMPLATE")
 
@@ -149,14 +160,17 @@ def snl_upload_file(urn_code):
 @with_appcontext
 def snl_list_files():
     """List files uploaded on SNL server."""
-    snl_repository = SNLRepository(
-        host=current_app.config.get("SONAR_APP_FTP_SNL_HOST"),
-        user=current_app.config.get("SONAR_APP_FTP_SNL_USER"),
-        password=current_app.config.get("SONAR_APP_FTP_SNL_PASSWORD"),
-        directory=current_app.config.get("SONAR_APP_FTP_SNL_PATH"),
-    )
+    snl_repository = create_snl_repository()
     snl_repository.connect()
-    snl_repository.client.walktree(".", lambda x: click.secho(x), lambda x: click.secho(x), lambda x: click.secho(x))
+    files = list(snl_repository.list_files())
+    snl_repository.close()
+
+    if not files:
+        click.secho("No file found on SNL server.", fg="yellow")
+        return
+
+    for path in files:
+        click.secho(path)
 
 
 @urn.command()

sonar/snl/ftp/__init__.py

@@ -3,41 +3,62 @@
 
 """SNL FTP repository."""
 
-from sftpretty import Connection
+import os
+from stat import S_ISDIR
+
+from paramiko import SSHClient
+from paramiko.hostkeys import HostKeyEntry
 
 
 class SNLRepository:
     """SNL FTP repository."""
 
-    def __init__(self, host, user, password, directory):
+    def __init__(self, host, user, password, directory, host_key=None):
         """Init class.
 
         :param host: FTP host.
         :param user: FTP user.
         :param password: FTP password.
         :param directory: Directory where files are stored.
+        :param host_key: Host key of the FTP server, as a `known_hosts` line.
         """
         self.host = host
         self.user = user
         self.password = password
         self.directory = directory
+        self.host_key = host_key
 
     def connect(self):
-        """Connect to FTP server and change directory."""
-        self.client = Connection(
+        """Connect to FTP server and change directory.
+
+        The host key must be known, otherwise the connection is rejected.
+        """
+        self.ssh = SSHClient()
+        self.ssh.load_system_host_keys()
+        if entry := HostKeyEntry.from_line((self.host_key or "").strip()):
+            for name in entry.hostnames:
+                self.ssh.get_host_keys().add(name, entry.key.get_name(), entry.key)
+        self.ssh.connect(
             self.host,
             username=self.user,
             password=self.password,
-            default_path=self.directory,
+            allow_agent=False,
+            look_for_keys=False,
+            timeout=30,
         )
+        self.client = self.ssh.open_sftp()
+        self.client.chdir(self.directory)
 
     def make_dir(self, pathname):
-        """Make new directory via FTP connection."""
-        self.client.mkdir(pathname)
+        """Make new directory via FTP connection, if it does not exist yet."""
+        try:
+            self.client.stat(pathname)
+        except FileNotFoundError:
+            self.client.mkdir(pathname, mode=0o777)
 
     def cwd(self, pathname):
         """Move to directory via FTP connection."""
-        self.client.cd(pathname)
+        self.client.chdir(pathname)
 
     def upload_file(self, file_path, file_name):
         """Upload file to SNL server via FTP connection.
@@ -46,6 +67,20 @@ def upload_file(self, file_path, file_name):
         """
         self.client.put(file_path, file_name)
 
+    def list_files(self, pathname="."):
+        """Recursively list files stored in a directory via FTP connection.
+
+        :param pathname: remote directory to walk through.
+        :returns: generator of remote file paths.
+        """
+        for attribute in self.client.listdir_attr(pathname):
+            path = os.path.join(pathname, attribute.filename)
+            if S_ISDIR(attribute.st_mode):
+                yield from self.list_files(path)
+            else:
+                yield path
+
     def close(self):
         """Close FTP connection."""
         self.client.close()
+        self.ssh.close()

tests/ui/documents/test_urn_cli.py

@@ -9,16 +9,17 @@
 from click.testing import CliRunner
 from invenio_pidstore.providers.base import BaseProvider
 
-from sonar.modules.documents.cli.urn import snl_upload_file
+from sonar.modules.documents.cli.urn import snl_list_files, snl_upload_file
 from sonar.snl.ftp import SNLRepository
 
 
-@mock.patch("sonar.snl.ftp.Connection", autospec=True)
-def test_snl_upload_file(mock_ftp_constructor, app, script_info, minimal_thesis_document_with_urn):
+@mock.patch("sonar.snl.ftp.SSHClient", autospec=True)
+def test_snl_upload_file(mock_ssh_constructor, app, script_info, minimal_thesis_document_with_urn):
     """Test upload file."""
     app.config["SONAR_APP_FTP_SNL_PATH"] = "/rero"
 
-    mock_ftp = mock_ftp_constructor.return_value
+    mock_ftp = mock_ssh_constructor.return_value.open_sftp.return_value
+    mock_ftp.stat.side_effect = FileNotFoundError
 
     repository = SNLRepository("snl_host", "user", "password", "snl_folder")
     repository.connect()
@@ -53,4 +54,22 @@ def test_snl_upload_file(mock_ftp_constructor, app, script_info, minimal_thesis_
             obj=script_info,
         )
         assert "Template of email to send to SNL:" in result.output
-        mock_ftp.mkdir.assert_called_with("/rero/rero-006-17")
+        mock_ftp.mkdir.assert_called_with("/rero/rero-006-17", mode=0o777)
+
+
+@mock.patch("sonar.snl.ftp.SNLRepository.list_files")
+@mock.patch("sonar.snl.ftp.SSHClient", autospec=True)
+def test_snl_list_files(mock_ssh_constructor, mock_list_files, app, script_info):
+    """Test listing of the files stored on the SNL server."""
+    mock_list_files.return_value = ["./rero-006-17/test.pdf", "./readme.txt"]
+
+    runner = CliRunner()
+    result = runner.invoke(snl_list_files, obj=script_info)
+
+    assert result.output == "./rero-006-17/test.pdf\n./readme.txt\n"
+
+    # empty server
+    mock_list_files.return_value = []
+    result = runner.invoke(snl_list_files, obj=script_info)
+
+    assert result.output == "No file found on SNL server.\n"

tests/unit/snl/__init__.py

@@ -0,0 +1,4 @@
+# SPDX-FileCopyrightText: Fondation RERO+
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""Tests unit snl."""

tests/unit/snl/ftp/__init__.py

@@ -0,0 +1,4 @@
+# SPDX-FileCopyrightText: Fondation RERO+
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""Tests unit snl ftp."""

tests/unit/snl/ftp/test_snl_repository.py

@@ -0,0 +1,102 @@
+# SPDX-FileCopyrightText: Fondation RERO+
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+"""Test SNL FTP repository."""
+
+from stat import S_IFDIR, S_IFREG
+from unittest import mock
+
+import pytest
+from paramiko import SFTPAttributes
+
+from sonar.snl.ftp import SNLRepository
+
+
+def sftp_attribute(filename, mode):
+    """Build a remote directory entry.
+
+    :param filename: name of the entry.
+    :param mode: stat mode of the entry.
+    :returns: SFTPAttributes object.
+    """
+    attribute = SFTPAttributes()
+    attribute.filename = filename
+    attribute.st_mode = mode
+    return attribute
+
+
+@pytest.fixture
+def repository():
+    """Return a repository connected to a mocked SFTP server."""
+    with mock.patch("sonar.snl.ftp.SSHClient", autospec=True):
+        repository = SNLRepository("snl_host", "user", "password", "/snl_folder")
+        repository.connect()
+        yield repository
+
+
+SNL_HOST_KEY = "snl_host ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPZqFuISvMFSw/cZXBVt/AnjFXbb0N+xbwx0ZMzn3x6h"
+
+
+@mock.patch("sonar.snl.ftp.SSHClient", autospec=True)
+def test_connect(mock_ssh_client):
+    """Test connection to the SNL server, host key taken from known_hosts."""
+    repository = SNLRepository("snl_host", "user", "password", "/snl_folder")
+    repository.connect()
+
+    repository.ssh.load_system_host_keys.assert_called_with()
+    repository.ssh.get_host_keys.return_value.add.assert_not_called()
+    repository.ssh.connect.assert_called_with(
+        "snl_host", username="user", password="password", allow_agent=False, look_for_keys=False, timeout=30
+    )
+    repository.client.chdir.assert_called_with("/snl_folder")
+
+
+@mock.patch("paramiko.SSHClient.open_sftp")
+@mock.patch("paramiko.SSHClient.connect")
+def test_connect_with_configured_host_key(mock_connect, mock_open_sftp):
+    """Test connection to the SNL server, host key given by the configuration."""
+    repository = SNLRepository("snl_host", "user", "password", "/snl_folder", host_key=f"\n{SNL_HOST_KEY}\n")
+    repository.connect()
+
+    host_keys = repository.ssh.get_host_keys().lookup("snl_host")
+    assert list(host_keys) == ["ssh-ed25519"]
+
+
+def test_make_dir(repository):
+    """Test directory creation, an existing directory is left untouched."""
+    repository.make_dir("/snl_folder/rero-006-17")
+    repository.client.mkdir.assert_not_called()
+
+    repository.client.stat.side_effect = FileNotFoundError
+    repository.make_dir("/snl_folder/rero-006-17")
+    repository.client.mkdir.assert_called_with("/snl_folder/rero-006-17", mode=0o777)
+
+
+def test_cwd(repository):
+    """Test directory change."""
+    repository.cwd("/snl_folder/rero-006-17")
+    repository.client.chdir.assert_called_with("/snl_folder/rero-006-17")
+
+
+def test_upload_file(repository):
+    """Test file upload."""
+    repository.upload_file("/tmp/test.pdf", "/snl_folder/rero-006-17/test.pdf")
+    repository.client.put.assert_called_with("/tmp/test.pdf", "/snl_folder/rero-006-17/test.pdf")
+
+
+def test_list_files(repository):
+    """Test recursive listing of the stored files."""
+    tree = {
+        ".": [sftp_attribute("rero-006-17", S_IFDIR), sftp_attribute("readme.txt", S_IFREG)],
+        "./rero-006-17": [sftp_attribute("test.pdf", S_IFREG)],
+    }
+    repository.client.listdir_attr.side_effect = lambda pathname: tree[pathname]
+
+    assert list(repository.list_files()) == ["./rero-006-17/test.pdf", "./readme.txt"]
+
+
+def test_close(repository):
+    """Test connection closing."""
+    repository.close()
+    repository.client.close.assert_called_with()
+    repository.ssh.close.assert_called_with()