Compare commits

..
Author SHA1 Message Date
ksmithandClaude Sonnet 5 b3cedcfa48 Fix connect-time username prompt never reaching SSH/RDP authentication
The username entered at the connect-time prompt (added for issue #21)
was updating SessionTab's own in-memory Profile copy, but
SshSessionBackend/RdpSessionBackend are constructed with -- and only
ever read from -- their own separate Profile copy on a worker thread,
which never saw that edit. Authentication was still built from the
original (blank) username regardless of what was typed into the
prompt.

SessionConnectOptions gains a username field, populated by SessionTab
on every connect attempt and threaded through the same way password
already is; both backends now prefer options.username over
profile().username. Covered by a new SSH regression test using an
exact-match fixture host that only succeeds for a specific
user@host target.

Bump version to v2026.9.16.5.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 08:40:34 -06:00
8 changed files with 93 additions and 6 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.21) cmake_minimum_required(VERSION 3.21)
project(OrbitHub VERSION 2026.9.16.4 LANGUAGES CXX) project(OrbitHub VERSION 2026.9.16.5 LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_STANDARD_REQUIRED ON)
+16
View File
@@ -193,6 +193,22 @@ Delivered:
told the user to go edit the profile instead of ever prompting inline. told the user to go edit the profile instead of ever prompting inline.
Removed; connect-time prompting is now the only username gate for Removed; connect-time prompting is now the only username gate for
SSH/RDP SSH/RDP
- Even with the three checks above gone, a username entered at the
connect-time prompt still never actually reached SSH or RDP
authentication: `SshSessionBackend`/`RdpSessionBackend` are constructed
with their own `Profile` copy up front (moved to a worker thread) and
read `profile().username` directly, which never sees `SessionTab`'s
later edit to its own in-memory profile once the user answers the
prompt. `SessionConnectOptions` (which already carries `password` the
same way) gained a `username` field, populated by `SessionTab` from
its profile copy on every connect attempt; both backends now prefer
`options.username` over `profile().username` when building the actual
connect target/auth call. Covered by a new SSH regression test
(`tests/fixtures/fake_ssh.sh`'s `requireuser` host only accepts an
exact `prompted-user@requireuser` target, so the test fails unless the
option, not the stale profile copy, is actually used) -- RDP has no
equivalent fake-server test harness, so that side relies on mirroring
the already-tested `m_activeOptions.password` pattern exactly
- Robustness fix: an unrecognized `FramebufferUpdate` rectangle encoding - Robustness fix: an unrecognized `FramebufferUpdate` rectangle encoding
used to abort the connection generically; `kAnnouncedEncodings` is now used to abort the connection generically; `kAnnouncedEncodings` is now
the single source of truth for what `SetEncodings` announces and what the single source of truth for what `SetEncodings` announces and what
+7 -1
View File
@@ -1640,7 +1640,13 @@ void RdpSessionBackend::workerMain()
const Profile& p = profile(); const Profile& p = profile();
const QString host = p.host.trimmed(); const QString host = p.host.trimmed();
QString username = p.username.trimmed(); // m_activeOptions.username carries a value prompted for at connect time
// (see SessionTab::requestConnectOptions()) when the saved profile's
// own username was blank; profile().username never sees that edit
// since this backend's Profile copy was captured at construction time.
QString username = m_activeOptions.username.trimmed().isEmpty()
? p.username.trimmed()
: m_activeOptions.username.trimmed();
QString domain = p.domain.trimmed(); QString domain = p.domain.trimmed();
if (domain.isEmpty()) { if (domain.isEmpty()) {
const int domainSeparator = username.indexOf(QLatin1Char('\\')); const int domainSeparator = username.indexOf(QLatin1Char('\\'));
+6
View File
@@ -12,6 +12,12 @@
class SessionConnectOptions class SessionConnectOptions
{ {
public: public:
// Only set when the profile's own username was blank and SessionTab
// prompted for one inline at connect time (see issue #21); empty means
// "use the backend's own profile().username" as before. SSH/RDP need
// this up front, unlike VNC's Apple auth which discovers the need for
// one mid-connection via usernameRequested()/provideUsername() instead.
QString username;
QString password; QString password;
QString privateKeyPath; QString privateKeyPath;
QString knownHostsPolicy; QString knownHostsPolicy;
+4
View File
@@ -998,6 +998,10 @@ void SessionTab::requestConnectOptions(
{ {
SessionConnectOptions baseOptions; SessionConnectOptions baseOptions;
baseOptions.knownHostsPolicy = m_profile.knownHostsPolicy; baseOptions.knownHostsPolicy = m_profile.knownHostsPolicy;
// The backend's own Profile copy was captured when it was constructed
// and never sees later edits to m_profile (e.g. the username prompt
// below) -- it has to travel through here instead.
baseOptions.username = m_profile.username.trimmed();
const bool isSsh = m_profile.protocol.compare(QStringLiteral("SSH"), Qt::CaseInsensitive) == 0; const bool isSsh = m_profile.protocol.compare(QStringLiteral("SSH"), Qt::CaseInsensitive) == 0;
const bool isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0; const bool isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0;
+9 -2
View File
@@ -388,9 +388,16 @@ bool SshSessionBackend::startSshProcess(const SessionConnectOptions& options)
<< QStringLiteral("PasswordAuthentication=no"); << QStringLiteral("PasswordAuthentication=no");
} }
const QString target = p.username.trimmed().isEmpty() // options.username carries a value prompted for at connect time (see
// SessionTab::requestConnectOptions()) when the saved profile's own
// username was blank; profile().username never sees that edit since
// the backend's Profile copy was captured at construction time.
const QString username = options.username.trimmed().isEmpty()
? p.username.trimmed()
: options.username.trimmed();
const QString target = username.isEmpty()
? p.host.trimmed() ? p.host.trimmed()
: QStringLiteral("%1@%2").arg(p.username.trimmed(), p.host.trimmed()); : QStringLiteral("%1@%2").arg(username, p.host.trimmed());
args << target; args << target;
m_process->setProcessEnvironment(environment); m_process->setProcessEnvironment(environment);
+16
View File
@@ -6,6 +6,22 @@
# host, optionally as user@host, as the final argument). # host, optionally as user@host, as the final argument).
for arg in "$@"; do for arg in "$@"; do
case "$arg" in case "$arg" in
prompted-user@requireuser)
# Only the exact user@host below is accepted -- used to prove a
# username supplied via SessionConnectOptions (prompted for at
# connect time because the saved profile's own username was
# blank) actually reaches the ssh command line, not just that
# *some* connection to this host succeeds.
echo "Welcome to the fake host."
while IFS= read -r line; do
echo "$line"
done
exit 0
;;
*@requireuser|requireuser)
echo "Permission denied (publickey,password)." >&2
exit 255
;;
*@succeed|succeed) *@succeed|succeed)
echo "Welcome to the fake host." echo "Welcome to the fake host."
# Stay alive echoing stdin back (simulates an interactive # Stay alive echoing stdin back (simulates an interactive
+34 -2
View File
@@ -21,6 +21,13 @@ Profile makeProfile(const QString& fixtureHost)
return profile; return profile;
} }
Profile makeBlankUsernameProfile(const QString& fixtureHost)
{
Profile profile = makeProfile(fixtureHost);
profile.username.clear();
return profile;
}
SessionConnectOptions makeOptions() SessionConnectOptions makeOptions()
{ {
SessionConnectOptions options; SessionConnectOptions options;
@@ -50,10 +57,12 @@ private slots:
void connectionRefusedReachesFailedState(); void connectionRefusedReachesFailedState();
void sendInputEchoesThroughOutputReceived(); void sendInputEchoesThroughOutputReceived();
void reconnectRestartsAndReachesConnectedAgain(); void reconnectRestartsAndReachesConnectedAgain();
void connectOptionsUsernameReachesProcessWhenProfileUsernameIsBlank();
private: private:
QString fixturePath() const; QString fixturePath() const;
void createBackend(const QString& fixtureHost); void createBackend(const QString& fixtureHost);
void createBackend(const Profile& profile);
std::unique_ptr<SshSessionBackend> m_backend; std::unique_ptr<SshSessionBackend> m_backend;
SessionState m_lastState = SessionState::Disconnected; SessionState m_lastState = SessionState::Disconnected;
@@ -69,8 +78,12 @@ QString TestSshSessionBackend::fixturePath() const
void TestSshSessionBackend::createBackend(const QString& fixtureHost) void TestSshSessionBackend::createBackend(const QString& fixtureHost)
{ {
m_backend = createBackend(makeProfile(fixtureHost));
std::make_unique<SshSessionBackend>(makeProfile(fixtureHost), fixturePath(), nullptr); }
void TestSshSessionBackend::createBackend(const Profile& profile)
{
m_backend = std::make_unique<SshSessionBackend>(profile, fixturePath(), nullptr);
connect(m_backend.get(), connect(m_backend.get(),
&SessionBackend::stateChanged, &SessionBackend::stateChanged,
this, this,
@@ -232,5 +245,24 @@ void TestSshSessionBackend::reconnectRestartsAndReachesConnectedAgain()
QTRY_COMPARE(m_lastState, SessionState::Connected); QTRY_COMPARE(m_lastState, SessionState::Connected);
} }
void TestSshSessionBackend::connectOptionsUsernameReachesProcessWhenProfileUsernameIsBlank()
{
// Regression test for a bug where a username entered at the
// connect-time prompt (SessionTab::requestConnectOptions(), for a
// profile with no saved username -- issue #21) never actually reached
// the ssh process: SshSessionBackend built its target purely from
// profile().username, which is a separate copy captured when the
// backend was constructed and never sees SessionTab's later edit.
// fixtures/fake_ssh.sh's "requireuser" host only accepts the exact
// target "prompted-user@requireuser", so this fails unless
// SessionConnectOptions::username is actually used.
createBackend(makeBlankUsernameProfile(QStringLiteral("requireuser")));
SessionConnectOptions options = makeOptions();
options.username = QStringLiteral("prompted-user");
m_backend->connectSession(options);
QTRY_COMPARE(m_lastState, SessionState::Connected);
}
QTEST_GUILESS_MAIN(TestSshSessionBackend) QTEST_GUILESS_MAIN(TestSshSessionBackend)
#include "test_ssh_session_backend.moc" #include "test_ssh_session_backend.moc"