FTP: Sftp improvements [STUD-79812] [STUD-72320]#550
Conversation
In certain scenrios, if SshException is thrown swallow it Add support for keyboard interactive authentification Add unittests
|
|
the sonar concerns are for code that is in the unittests |
There was a problem hiding this comment.
Pull request overview
This PR improves the SFTP session in the FTP activities package by (1) adding keyboard-interactive authentication so servers configured with only publickey+keyboard-interactive can authenticate using the supplied password, and (2) tolerating bare SshException thrown by some SFTP servers from Exists() calls by treating it as "not found" rather than propagating. It also fixes a "passphare" → "passphrase" typo in resources, relaxes the no-credentials validation when UseSftp is set, and adds new unit tests for SftpSession, FileExists, and DirectoryExists activities.
Changes:
- Always register a
KeyboardInteractiveAuthenticationMethodand respond to non-echo prompts with the configured password. - Introduce
ClientExists/SafeExistsand route every_sftpClient.Exists(...)call throughSafeExiststo swallow bareSshException. - Add new test fixtures (with a
TestsHelper.FlattenandCopyBoolhelper) and fix the "passphrase" typo in both.resx/.Designer.cspairs; loosenWithFtpSessioncredential validation for SFTP.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Activities/FTP/UiPath.FTP/SftpSession.cs | Adds keyboard-interactive auth method and the ClientExists/SafeExists wrapper used by all Exists callers. |
| Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.resx | Fixes "passphare" → "passphrase" in error message. |
| Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.Designer.cs | Regenerated comment for the corrected resource. |
| Activities/FTP/UiPath.FTP.Activities/WithFtpSession.cs | Skips the password/cert required check when UseSftp is true. |
| Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.resx | Same "passphrase" typo fix in activities resources. |
| Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs | Regenerated comment for the corrected resource. |
| Activities/FTP/UiPath.FTP.Tests/TestsHelper.cs | Adds CopyBool and Flatten helpers used by new activity tests. |
| Activities/FTP/UiPath.FTP.Tests/SessionsTests/SftpSessionTests.cs | New tests for keyboard-interactive registration/handler and SafeExists exception handling (uses reflection on SSH.NET internals). |
| Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/FileExistsTests.cs | New activity-level tests for FileExists against a mocked IFtpSession. |
| Activities/FTP/UiPath.FTP.Tests/ActivitiesTests/DirectoryExistsTests.cs | New activity-level tests for DirectoryExists, structurally duplicated from FileExistsTests. |
Files not reviewed (2)
- Activities/FTP/UiPath.FTP.Activities/Properties/UiPath.FTP.Activities.Designer.cs: Language not supported
- Activities/FTP/UiPath.FTP/Properties/UiPath.FTP.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
Activities/FTP/UiPath.FTP/SftpSession.cs:71
- Since
KeyboardInteractiveAuthenticationMethodis now unconditionally added toauthMethodsabove,authMethods.Countcan never be zero, so this guard and the correspondingNoValidAuthenticationMethodexception are dead code. Either remove the guard, or keep the constructor-side validation meaningful by only adding the keyboard-interactive method when a password is actually configured (otherwise the handler will respond withstring.Emptyto every non-echo prompt, which is unlikely to ever succeed and is misleading at the server-side log level).
authMethods.Add(kbiMethod);
//Throw an error if we ended up with no authentication method
if (authMethods.Count == 0)
{
throw new ArgumentNullException(Resources.NoValidAuthenticationMethod);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AlexMarinescuUiPath
left a comment
There was a problem hiding this comment.
The old code threw [NoValidAuthenticationMethod] early if you had no password and no private key. The new code skips this check entirely when [UseSftp] is true:
The problem: If someone configures an SFTP session with [UseSftp = true] but forgets to set any credential (no password, no key), the activity no longer catches this upfront. Instead it proceeds to connect, the kbi handler responds with [string.Empty], and the server rejects it with some opaque SSH authentication error.
The user gets a confusing server-side error instead of the clear message "No valid authentication method found: You need to supply either Private Key file or Password."
Better approach: Keep the validation for everyone. The kbi path already uses [ftpConfiguration.Password], so if that's set, the existing check passes anyway. You only need kbi when a password IS configured but the server doesn't accept the password auth method — in that case the validation already passes. So the [!UseSftp] escape hatch is unnecessary.


In certain scenrios, if SshException is thrown swallow it
Add support for keyboard interactive authentification
Add unittests
https://uipath.atlassian.net/browse/STUD-72320
https://uipath.atlassian.net/browse/STUD-79812
Simple support is added for keyboard interactive authentication. The password or SecuredPassword from the card will be used for authentication. Custom prompts or 2FA Auth are not supported at the moment.
For testing purposes: the following scenario should be used: have a server that has supports only publickey and keyboard-interactive. If the password is correct, with previous version of the package it will fail to connect;with the new version it will connect.