FEATURE (ftp): Get rid of passive mode

2026-04-06 00:32:03 +02:00 · 2025-12-14 00:01:21 +03:00
parent 51d7fe54d0
commit 422b44dfdc
8 changed files with 62 additions and 74 deletions
--- a/backend/internal/features/storages/controller_test.go
+++ b/backend/internal/features/storages/controller_test.go
@@ -748,13 +748,12 @@ func Test_StorageSensitiveDataLifecycle_AllTypes(t *testing.T) {
 					Type:        StorageTypeFTP,
 					Name:        "Test FTP Storage",
 					FTPStorage: &ftp_storage.FTPStorage{
-						Host:        "ftp.example.com",
-						Port:        21,
-						Username:    "testuser",
-						Password:    "original-password",
-						UseSSL:      false,
-						PassiveMode: true,
-						Path:        "/backups",
+						Host:     "ftp.example.com",
+						Port:     21,
+						Username: "testuser",
+						Password: "original-password",
+						UseSSL:   false,
+						Path:     "/backups",
 					},
 				}
 			},
@@ -765,13 +764,12 @@ func Test_StorageSensitiveDataLifecycle_AllTypes(t *testing.T) {
 					Type:        StorageTypeFTP,
 					Name:        "Updated FTP Storage",
 					FTPStorage: &ftp_storage.FTPStorage{
-						Host:        "ftp2.example.com",
-						Port:        2121,
-						Username:    "testuser2",
-						Password:    "",
-						UseSSL:      true,
-						PassiveMode: false,
-						Path:        "/backups2",
+						Host:     "ftp2.example.com",
+						Port:     2121,
+						Username: "testuser2",
+						Password: "",
+						UseSSL:   true,
+						Path:     "/backups2",
 					},
 				}
 			},
--- a/backend/internal/features/storages/model_test.go
+++ b/backend/internal/features/storages/model_test.go
@@ -136,14 +136,13 @@ func Test_Storage_BasicOperations(t *testing.T) {
 		{
 			name: "FTPStorage",
 			storage: &ftp_storage.FTPStorage{
-				StorageID:   uuid.New(),
-				Host:        "localhost",
-				Port:        ftpPort,
-				Username:    "testuser",
-				Password:    "testpassword",
-				UseSSL:      false,
-				PassiveMode: true,
-				Path:        "test-files",
+				StorageID: uuid.New(),
+				Host:      "localhost",
+				Port:      ftpPort,
+				Username:  "testuser",
+				Password:  "testpassword",
+				UseSSL:    false,
+				Path:      "test-files",
 			},
 		},
 	}
--- a/backend/internal/features/storages/models/ftp/model.go
+++ b/backend/internal/features/storages/models/ftp/model.go
@@ -16,8 +16,9 @@ import (
 )

 const (
-	ftpConnectTimeout = 30 * time.Second
-	ftpChunkSize      = 16 * 1024 * 1024
+	ftpConnectTimeout     = 30 * time.Second
+	ftpTestConnectTimeout = 10 * time.Second
+	ftpChunkSize          = 16 * 1024 * 1024
 )

 type FTPStorage struct {
@@ -29,7 +30,6 @@ type FTPStorage struct {
 	Path          string    `json:"path"          gorm:"type:text;column:path"`
 	UseSSL        bool      `json:"useSsl"        gorm:"not null;default:false;column:use_ssl"`
 	SkipTLSVerify bool      `json:"skipTlsVerify" gorm:"not null;default:false;column:skip_tls_verify"`
-	PassiveMode   bool      `json:"passiveMode"   gorm:"not null;default:true;column:passive_mode"`
 }

 func (f *FTPStorage) TableName() string {
@@ -51,7 +51,7 @@ func (f *FTPStorage) SaveFile(

 	logger.Info("Starting to save file to FTP storage", "fileId", fileID.String(), "host", f.Host)

-	conn, err := f.connect(encryptor)
+	conn, err := f.connect(encryptor, ftpConnectTimeout)
 	if err != nil {
 		logger.Error("Failed to connect to FTP", "fileId", fileID.String(), "error", err)
 		return fmt.Errorf("failed to connect to FTP: %w", err)
@@ -114,7 +114,7 @@ func (f *FTPStorage) GetFile(
 	encryptor encryption.FieldEncryptor,
 	fileID uuid.UUID,
 ) (io.ReadCloser, error) {
-	conn, err := f.connect(encryptor)
+	conn, err := f.connect(encryptor, ftpConnectTimeout)
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to FTP: %w", err)
 	}
@@ -134,7 +134,7 @@ func (f *FTPStorage) GetFile(
 }

 func (f *FTPStorage) DeleteFile(encryptor encryption.FieldEncryptor, fileID uuid.UUID) error {
-	conn, err := f.connect(encryptor)
+	conn, err := f.connect(encryptor, ftpConnectTimeout)
 	if err != nil {
 		return fmt.Errorf("failed to connect to FTP: %w", err)
 	}
@@ -175,7 +175,10 @@ func (f *FTPStorage) Validate(encryptor encryption.FieldEncryptor) error {
 }

 func (f *FTPStorage) TestConnection(encryptor encryption.FieldEncryptor) error {
-	conn, err := f.connect(encryptor)
+	ctx, cancel := context.WithTimeout(context.Background(), ftpTestConnectTimeout)
+	defer cancel()
+
+	conn, err := f.connectWithContext(ctx, encryptor, ftpTestConnectTimeout)
 	if err != nil {
 		return fmt.Errorf("failed to connect to FTP: %w", err)
 	}
@@ -214,7 +217,6 @@ func (f *FTPStorage) Update(incoming *FTPStorage) {
 	f.Username = incoming.Username
 	f.UseSSL = incoming.UseSSL
 	f.SkipTLSVerify = incoming.SkipTLSVerify
-	f.PassiveMode = incoming.PassiveMode
 	f.Path = incoming.Path

 	if incoming.Password != "" {
@@ -222,7 +224,18 @@ func (f *FTPStorage) Update(incoming *FTPStorage) {
 	}
 }

-func (f *FTPStorage) connect(encryptor encryption.FieldEncryptor) (*ftp.ServerConn, error) {
+func (f *FTPStorage) connect(
+	encryptor encryption.FieldEncryptor,
+	timeout time.Duration,
+) (*ftp.ServerConn, error) {
+	return f.connectWithContext(context.Background(), encryptor, timeout)
+}
+
+func (f *FTPStorage) connectWithContext(
+	ctx context.Context,
+	encryptor encryption.FieldEncryptor,
+	timeout time.Duration,
+) (*ftp.ServerConn, error) {
 	password, err := encryptor.Decrypt(f.StorageID, f.Password)
 	if err != nil {
 		return nil, fmt.Errorf("failed to decrypt FTP password: %w", err)
@@ -230,6 +243,9 @@ func (f *FTPStorage) connect(encryptor encryption.FieldEncryptor) (*ftp.ServerCo

 	address := fmt.Sprintf("%s:%d", f.Host, f.Port)

+	dialCtx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+
 	var conn *ftp.ServerConn
 	if f.UseSSL {
 		tlsConfig := &tls.Config{
@@ -237,11 +253,11 @@ func (f *FTPStorage) connect(encryptor encryption.FieldEncryptor) (*ftp.ServerCo
 			InsecureSkipVerify: f.SkipTLSVerify,
 		}
 		conn, err = ftp.Dial(address,
-			ftp.DialWithTimeout(ftpConnectTimeout),
+			ftp.DialWithContext(dialCtx),
 			ftp.DialWithExplicitTLS(tlsConfig),
 		)
 	} else {
-		conn, err = ftp.Dial(address, ftp.DialWithTimeout(ftpConnectTimeout))
+		conn, err = ftp.Dial(address, ftp.DialWithContext(dialCtx))
 	}
 	if err != nil {
 		return nil, fmt.Errorf("failed to dial FTP server: %w", err)
--- a/backend/migrations/20251213204730_remove_ftp_passive_mode.sql
+++ b/backend/migrations/20251213204730_remove_ftp_passive_mode.sql
@@ -0,0 +1,15 @@
+-- +goose Up
+-- +goose StatementBegin
+
+ALTER TABLE ftp_storages
+    DROP COLUMN passive_mode;
+
+-- +goose StatementEnd
+
+-- +goose Down
+-- +goose StatementBegin
+
+ALTER TABLE ftp_storages
+    ADD COLUMN passive_mode BOOLEAN NOT NULL DEFAULT TRUE;
+
+-- +goose StatementEnd
--- a/frontend/src/entity/storages/models/FTPStorage.ts
+++ b/frontend/src/entity/storages/models/FTPStorage.ts
@@ -5,6 +5,5 @@ export interface FTPStorage {
  password: string;
  useSsl: boolean;
  skipTlsVerify?: boolean;
-  passiveMode: boolean;
  path?: string;
 }
--- a/frontend/src/features/storages/ui/edit/EditStorageComponent.tsx
+++ b/frontend/src/features/storages/ui/edit/EditStorageComponent.tsx
@@ -142,7 +142,6 @@ export function EditStorageComponent({
        username: '',
        password: '',
        useSsl: false,
-        passiveMode: true,
        path: '',
      };
    }
--- a/frontend/src/features/storages/ui/edit/storages/EditFTPStorageComponent.tsx
+++ b/frontend/src/features/storages/ui/edit/storages/EditFTPStorageComponent.tsx
@@ -11,8 +11,7 @@ interface Props {
 }

 export function EditFTPStorageComponent({ storage, setStorage, setUnsaved }: Props) {
-  const hasAdvancedValues =
-    !!storage?.ftpStorage?.skipTlsVerify || storage?.ftpStorage?.passiveMode === false;
+  const hasAdvancedValues = !!storage?.ftpStorage?.skipTlsVerify;
  const [showAdvanced, setShowAdvanced] = useState(hasAdvancedValues);

  return (
@@ -190,36 +189,6 @@ export function EditFTPStorageComponent({ storage, setStorage, setUnsaved }: Pro

      {showAdvanced && (
        <>
-          <div className="mb-1 flex w-full flex-col items-start sm:flex-row sm:items-center">
-            <div className="mb-1 min-w-[110px] sm:mb-0">Passive mode</div>
-            <div className="flex items-center">
-              <Checkbox
-                checked={storage?.ftpStorage?.passiveMode !== false}
-                onChange={(e) => {
-                  if (!storage?.ftpStorage) return;
-
-                  setStorage({
-                    ...storage,
-                    ftpStorage: {
-                      ...storage.ftpStorage,
-                      passiveMode: e.target.checked,
-                    },
-                  });
-                  setUnsaved();
-                }}
-              >
-                Use passive mode
-              </Checkbox>
-
-              <Tooltip
-                className="cursor-pointer"
-                title="Passive mode is recommended for most firewalls and NAT configurations. Disable only if you have issues connecting."
-              >
-                <InfoCircleOutlined className="ml-2" style={{ color: 'gray' }} />
-              </Tooltip>
-            </div>
-          </div>
-
          {storage?.ftpStorage?.useSsl && (
            <div className="mb-1 flex w-full flex-col items-start sm:flex-row sm:items-center">
              <div className="mb-1 min-w-[110px] sm:mb-0">Skip TLS verify</div>
--- a/frontend/src/features/storages/ui/show/storages/ShowFTPStorageComponent.tsx
+++ b/frontend/src/features/storages/ui/show/storages/ShowFTPStorageComponent.tsx
@@ -43,13 +43,6 @@ export function ShowFTPStorageComponent({ storage }: Props) {
          Enabled
        </div>
      )}
-
-      {storage?.ftpStorage?.passiveMode === false && (
-        <div className="mb-1 flex items-center">
-          <div className="min-w-[110px]">Passive mode</div>
-          Disabled
-        </div>
-      )}
    </>
  );
 }