diff --git a/CMakeLists.txt b/CMakeLists.txt index 5612794..4e3fc41 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ 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_REQUIRED ON) diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 2dabaa3..49bcba2 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -193,6 +193,22 @@ Delivered: told the user to go edit the profile instead of ever prompting inline. Removed; connect-time prompting is now the only username gate for 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 used to abort the connection generically; `kAnnouncedEncodings` is now the single source of truth for what `SetEncodings` announces and what diff --git a/src/rdp_session_backend.cpp b/src/rdp_session_backend.cpp index 9da7980..781e50e 100644 --- a/src/rdp_session_backend.cpp +++ b/src/rdp_session_backend.cpp @@ -1640,7 +1640,13 @@ void RdpSessionBackend::workerMain() const Profile& p = profile(); 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(); if (domain.isEmpty()) { const int domainSeparator = username.indexOf(QLatin1Char('\\')); diff --git a/src/session_backend.h b/src/session_backend.h index a5c6b96..79d2c30 100644 --- a/src/session_backend.h +++ b/src/session_backend.h @@ -12,6 +12,12 @@ class SessionConnectOptions { 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 privateKeyPath; QString knownHostsPolicy; diff --git a/src/session_tab.cpp b/src/session_tab.cpp index 891d7ee..0cac7cf 100644 --- a/src/session_tab.cpp +++ b/src/session_tab.cpp @@ -998,6 +998,10 @@ void SessionTab::requestConnectOptions( { SessionConnectOptions baseOptions; 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 isRdp = m_profile.protocol.compare(QStringLiteral("RDP"), Qt::CaseInsensitive) == 0; diff --git a/src/ssh_session_backend.cpp b/src/ssh_session_backend.cpp index 5343b9f..ad70344 100644 --- a/src/ssh_session_backend.cpp +++ b/src/ssh_session_backend.cpp @@ -388,9 +388,16 @@ bool SshSessionBackend::startSshProcess(const SessionConnectOptions& options) << 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() - : QStringLiteral("%1@%2").arg(p.username.trimmed(), p.host.trimmed()); + : QStringLiteral("%1@%2").arg(username, p.host.trimmed()); args << target; m_process->setProcessEnvironment(environment); diff --git a/tests/fixtures/fake_ssh.sh b/tests/fixtures/fake_ssh.sh index 22f2e89..3379128 100755 --- a/tests/fixtures/fake_ssh.sh +++ b/tests/fixtures/fake_ssh.sh @@ -6,6 +6,22 @@ # host, optionally as user@host, as the final argument). for arg in "$@"; do 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) echo "Welcome to the fake host." # Stay alive echoing stdin back (simulates an interactive diff --git a/tests/test_ssh_session_backend.cpp b/tests/test_ssh_session_backend.cpp index 367e1c4..c506ce5 100644 --- a/tests/test_ssh_session_backend.cpp +++ b/tests/test_ssh_session_backend.cpp @@ -21,6 +21,13 @@ Profile makeProfile(const QString& fixtureHost) return profile; } +Profile makeBlankUsernameProfile(const QString& fixtureHost) +{ + Profile profile = makeProfile(fixtureHost); + profile.username.clear(); + return profile; +} + SessionConnectOptions makeOptions() { SessionConnectOptions options; @@ -50,10 +57,12 @@ private slots: void connectionRefusedReachesFailedState(); void sendInputEchoesThroughOutputReceived(); void reconnectRestartsAndReachesConnectedAgain(); + void connectOptionsUsernameReachesProcessWhenProfileUsernameIsBlank(); private: QString fixturePath() const; void createBackend(const QString& fixtureHost); + void createBackend(const Profile& profile); std::unique_ptr m_backend; SessionState m_lastState = SessionState::Disconnected; @@ -69,8 +78,12 @@ QString TestSshSessionBackend::fixturePath() const void TestSshSessionBackend::createBackend(const QString& fixtureHost) { - m_backend = - std::make_unique(makeProfile(fixtureHost), fixturePath(), nullptr); + createBackend(makeProfile(fixtureHost)); +} + +void TestSshSessionBackend::createBackend(const Profile& profile) +{ + m_backend = std::make_unique(profile, fixturePath(), nullptr); connect(m_backend.get(), &SessionBackend::stateChanged, this, @@ -232,5 +245,24 @@ void TestSshSessionBackend::reconnectRestartsAndReachesConnectedAgain() 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) #include "test_ssh_session_backend.moc"