Skip to content

Commit b315b42

Browse files
vdavidvdavid
committed
SMB: Show login form when direct conn needs creds
- When "Connect directly" fails due to missing/wrong credentials, show `NetworkLoginForm` inline in the pane instead of an opaque error toast - `upgrade_to_smb_volume` now returns structured `UpgradeResult` (success / credentials needed / network error) instead of a flat error string - New `upgrade_to_smb_volume_with_credentials` command for the login form to call with explicit credentials + optional Keychain save - Fix mDNS service name resolution: `statfs` can return names like `Naspolya._smb._tcp.local` which are not DNS-resolvable — new `resolve_server_address` resolves them to IPs via discovered hosts. Fixes all three upgrade paths (startup, mount-time, manual). - Fix volume display name: show `naspi on Naspolya` instead of `naspi on Naspolya._smb._tcp.local` - `NetworkLoginForm` gains `defaultConnectionMode` prop so the upgrade flow defaults to credentials (not guest) while still offering "Connect as guest"
1 parent 6308476 commit b315b42

13 files changed

Lines changed: 441 additions & 45 deletions

File tree

‎apps/desktop/src-tauri/src/commands/CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ immediately to business-logic modules. No significant logic lives here.
1313
| `volumes.rs` | Volume management (macOS) | `list_volumes`, `get_default_volume_id`, `get_volume_space`, `resolve_path_volume` (statfs-based, no volume enumeration) |
1414
| `volumes_linux.rs` | Volume management (Linux) | Same interface as `volumes.rs`, delegates to `volumes_linux` module |
1515
| `mtp.rs` | MTP devices | Full MTP command surface (connect, disconnect, list, download, upload, delete, rename, move, scan) |
16-
| `network.rs` | SMB/network shares | Discovery, share listing, keychain, mounting. |
16+
| `network.rs` | SMB/network shares | Discovery, share listing, keychain, mounting, direct-connection upgrade (with inline login form for credentials). |
1717
| `font_metrics.rs` | Font metrics cache | `store_font_metrics`, `has_font_metrics` |
1818
| `icons.rs` | File icons | `get_icons`, `refresh_directory_icons`, cache clear |
1919
| `rename.rs` | Rename / trash | `move_to_trash` (delegates to `write_operations::trash::move_to_trash_sync`), `check_rename_permission`, `check_rename_validity`, `rename_file`. `rename_file` calls `notify_mutation` after success to update the listing cache (both local and volume-aware paths). |

‎apps/desktop/src-tauri/src/commands/network.rs‎

Lines changed: 258 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,30 @@ use crate::network::{
66
update_host_resolution,
77
};
88

9+
/// Result of an SMB volume upgrade attempt.
10+
#[derive(serde::Serialize)]
11+
#[serde(tag = "status", rename_all = "camelCase")]
12+
pub enum UpgradeResult {
13+
/// Upgrade succeeded — volume now uses direct smb2.
14+
Success,
15+
/// Credentials needed — frontend should show login form.
16+
CredentialsNeeded {
17+
server: String,
18+
share: String,
19+
port: u16,
20+
/// Friendly display name for the server (mDNS hostname or IP).
21+
display_name: String,
22+
/// Username hint from stored credentials or the OS mount.
23+
username_hint: Option<String>,
24+
/// Optional message explaining why credentials are needed.
25+
message: Option<String>,
26+
},
27+
/// Non-auth error (DNS, network, unreachable).
28+
NetworkError {
29+
message: String,
30+
},
31+
}
32+
933
/// Gets all currently discovered network hosts.
1034
#[tauri::command]
1135
pub fn list_network_hosts() -> Vec<NetworkHost> {
@@ -338,14 +362,17 @@ pub(crate) async fn register_smb_volume(
338362
use crate::file_system::volume::smb::connect_smb_volume;
339363
use std::sync::Arc;
340364

365+
// Resolve mDNS service names (like "Naspolya._smb._tcp.local") to an IP
366+
let resolved_server = resolve_server_address(server);
367+
341368
log::debug!(
342369
"Establishing smb2 connection for SmbVolume: {}:{}/{}",
343-
server,
370+
resolved_server,
344371
port,
345372
share
346373
);
347374

348-
match connect_smb_volume(share, mount_path, server, share, username, password, port).await {
375+
match connect_smb_volume(share, mount_path, &resolved_server, share, username, password, port).await {
349376
Ok(volume) => {
350377
let volume_id = crate::file_system::volume::path_to_id(mount_path);
351378
// Use register (overwrite) so SmbVolume always wins over any
@@ -367,12 +394,13 @@ pub(crate) async fn register_smb_volume(
367394

368395
/// Upgrades an existing OS-mounted SMB volume to use a direct smb2 connection.
369396
///
370-
/// Extracts server/share/username from `statfs`, connects via smb2,
371-
/// and replaces the `LocalPosixVolume` with an `SmbVolume` in `VolumeManager`.
397+
/// Extracts server/share/username from `statfs`, tries stored credentials,
398+
/// and either upgrades to `SmbVolume` or returns `CredentialsNeeded` so
399+
/// the frontend can show a login form.
372400
///
373401
/// Called from the "Connect directly for faster access" UI action.
374402
#[tauri::command]
375-
pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<String, String> {
403+
pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<UpgradeResult, String> {
376404
use crate::file_system::get_volume_manager;
377405
#[cfg(target_os = "macos")]
378406
use crate::volumes::get_smb_mount_info;
@@ -387,7 +415,7 @@ pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<String, String>
387415

388416
// Check if already an SmbVolume
389417
if volume.smb_connection_state().is_some() {
390-
return Ok("direct".to_string());
418+
return Ok(UpgradeResult::Success);
391419
}
392420

393421
// Extract SMB connection info from statfs
@@ -409,31 +437,181 @@ pub async fn upgrade_to_smb_volume(volume_id: String) -> Result<String, String>
409437
// Try to get credentials from Keychain. The mount source has the IP, but Cmdr
410438
// stores Keychain credentials keyed by hostname (from mDNS). Try both.
411439
let hostname = resolve_ip_to_hostname(&info.server);
440+
let display_name = friendly_server_name(&info.server);
412441
let creds = get_keychain_password(&info.server, hostname.as_deref(), &info.share).await;
413442

414443
match &creds {
415444
Some((u, _)) => log::info!("Found Keychain credentials for user={}", u),
416-
None => log::info!("No Keychain credentials found, trying guest access"),
445+
None => {
446+
log::info!("No stored credentials found, requesting credentials from user");
447+
return Ok(UpgradeResult::CredentialsNeeded {
448+
server: info.server,
449+
share: info.share,
450+
port: info.port,
451+
display_name,
452+
username_hint: info.username,
453+
message: None,
454+
});
455+
}
417456
}
418457

419458
let (username, password) = match &creds {
420459
Some((u, p)) => (Some(u.as_str()), Some(p.as_str())),
421-
None => (None, None),
460+
None => unreachable!(),
422461
};
423462

424-
register_smb_volume(&info.server, &info.share, &mount_path, username, password, info.port).await;
463+
// Try connecting with stored credentials
464+
let result = try_smb_upgrade(
465+
&info.server,
466+
&info.share,
467+
&mount_path,
468+
username,
469+
password,
470+
info.port,
471+
&volume_id,
472+
)
473+
.await;
425474

426-
// Check if it worked
427-
if let Some(vol) = manager.get(&volume_id)
428-
&& vol.smb_connection_state().is_some()
429-
{
430-
return Ok("direct".to_string());
475+
match result {
476+
Ok(()) => Ok(UpgradeResult::Success),
477+
Err(UpgradeError::Auth) => {
478+
log::info!("Stored credentials didn't work, requesting new credentials");
479+
Ok(UpgradeResult::CredentialsNeeded {
480+
server: info.server,
481+
share: info.share,
482+
port: info.port,
483+
display_name,
484+
username_hint: username.map(|s| s.to_string()),
485+
message: Some("Stored credentials didn't work".to_string()),
486+
})
487+
}
488+
Err(UpgradeError::Network(msg)) => Ok(UpgradeResult::NetworkError { message: msg }),
431489
}
490+
}
491+
492+
/// Upgrades an existing OS-mounted SMB volume using explicit credentials.
493+
///
494+
/// Called after the user fills in the login form shown by `upgrade_to_smb_volume`.
495+
#[tauri::command]
496+
pub async fn upgrade_to_smb_volume_with_credentials(
497+
volume_id: String,
498+
username: Option<String>,
499+
password: Option<String>,
500+
remember_in_keychain: bool,
501+
) -> Result<UpgradeResult, String> {
502+
use crate::file_system::get_volume_manager;
503+
#[cfg(target_os = "macos")]
504+
use crate::volumes::get_smb_mount_info;
505+
#[cfg(target_os = "linux")]
506+
use crate::volumes_linux::get_smb_mount_info;
432507

433-
Err(format!(
434-
"Failed to establish direct smb2 connection to {}/{}",
435-
info.server, info.share
436-
))
508+
let manager = get_volume_manager();
509+
510+
let volume = manager.get(&volume_id).ok_or("Volume not found")?;
511+
let mount_path = volume.root().to_string_lossy().to_string();
512+
513+
if volume.smb_connection_state().is_some() {
514+
return Ok(UpgradeResult::Success);
515+
}
516+
517+
let info = get_smb_mount_info(&mount_path).ok_or_else(|| {
518+
format!(
519+
"Can't determine SMB server info for {}. Is this an SMB mount?",
520+
mount_path
521+
)
522+
})?;
523+
524+
let hostname = resolve_ip_to_hostname(&info.server);
525+
let display_name = friendly_server_name(&info.server);
526+
527+
let result = try_smb_upgrade(
528+
&info.server,
529+
&info.share,
530+
&mount_path,
531+
username.as_deref(),
532+
password.as_deref(),
533+
info.port,
534+
&volume_id,
535+
)
536+
.await;
537+
538+
match result {
539+
Ok(()) => {
540+
// Save credentials on success if requested
541+
if remember_in_keychain
542+
&& let (Some(u), Some(p)) = (&username, &password) {
543+
let server_key = hostname.as_deref().unwrap_or(&info.server);
544+
if let Err(e) = keychain::save_credentials(
545+
server_key,
546+
Some(&info.share),
547+
u,
548+
p,
549+
) {
550+
log::warn!("Couldn't save credentials to Keychain: {}", e);
551+
}
552+
}
553+
Ok(UpgradeResult::Success)
554+
}
555+
Err(UpgradeError::Auth) => Ok(UpgradeResult::CredentialsNeeded {
556+
server: info.server,
557+
share: info.share,
558+
port: info.port,
559+
display_name,
560+
username_hint: username,
561+
message: Some("Invalid username or password".to_string()),
562+
}),
563+
Err(UpgradeError::Network(msg)) => Ok(UpgradeResult::NetworkError { message: msg }),
564+
}
565+
}
566+
567+
/// Internal error type for upgrade attempts, distinguishing auth from network failures.
568+
enum UpgradeError {
569+
Auth,
570+
Network(String),
571+
}
572+
573+
/// Attempts the smb2 connection and registers the volume. Returns `Ok(())` on success.
574+
async fn try_smb_upgrade(
575+
server: &str,
576+
share: &str,
577+
mount_path: &str,
578+
username: Option<&str>,
579+
password: Option<&str>,
580+
port: u16,
581+
volume_id: &str,
582+
) -> Result<(), UpgradeError> {
583+
use crate::file_system::get_volume_manager;
584+
use crate::file_system::volume::smb::connect_smb_volume;
585+
use crate::network::smb_util::is_auth_error;
586+
use std::sync::Arc;
587+
588+
// Resolve mDNS service names to connectable addresses
589+
let resolved_server = resolve_server_address(server);
590+
let display = friendly_server_name(server);
591+
592+
match connect_smb_volume(share, mount_path, &resolved_server, share, username, password, port).await {
593+
Ok(volume) => {
594+
get_volume_manager().register(volume_id, Arc::new(volume));
595+
log::info!("Registered SmbVolume for {} (id={})", mount_path, volume_id);
596+
Ok(())
597+
}
598+
Err(e) => {
599+
if is_auth_error(&e) {
600+
Err(UpgradeError::Auth)
601+
} else {
602+
log::warn!(
603+
"Failed to establish smb2 connection for {}/{}: {}",
604+
resolved_server,
605+
share,
606+
e
607+
);
608+
Err(UpgradeError::Network(format!(
609+
"Can't connect to {} — check that it's reachable on your network",
610+
display
611+
)))
612+
}
613+
}
614+
}
437615
}
438616

439617
/// Looks up the mDNS hostname for an IP address from discovered hosts.
@@ -450,6 +628,68 @@ pub(crate) fn resolve_ip_to_hostname(ip: &str) -> Option<String> {
450628
None
451629
}
452630

631+
/// Resolves a server address from `statfs` to a connectable address.
632+
///
633+
/// `statfs` can return different formats depending on how the mount was created:
634+
/// - An IP address like `192.168.1.111` — usable as-is
635+
/// - A DNS hostname like `fileserver.corp.example.com` — usable as-is
636+
/// - An mDNS service name like `Naspolya._smb._tcp.local` — NOT resolvable by DNS,
637+
/// must be resolved to an IP via the mDNS discovery state
638+
///
639+
/// Returns the resolved IP if possible, otherwise the original string.
640+
pub(crate) fn resolve_server_address(server: &str) -> String {
641+
// Detect mDNS service names (contain "._tcp" or "._udp")
642+
if !server.contains("._tcp") && !server.contains("._udp") {
643+
return server.to_string();
644+
}
645+
646+
// Extract the service/display name (everything before the first "._")
647+
let service_name = server.split("._").next().unwrap_or(server);
648+
649+
// Look up the discovered host by name (case-insensitive)
650+
let hosts = get_discovered_hosts();
651+
for host in &hosts {
652+
if host.name.eq_ignore_ascii_case(service_name) {
653+
if let Some(ref ip) = host.ip_address {
654+
log::debug!(
655+
"Resolved mDNS service name {} to IP {}",
656+
server,
657+
ip
658+
);
659+
return ip.clone();
660+
}
661+
// Host found but no IP yet — try the hostname
662+
if let Some(ref hostname) = host.hostname {
663+
log::debug!(
664+
"Resolved mDNS service name {} to hostname {}",
665+
server,
666+
hostname
667+
);
668+
return hostname.clone();
669+
}
670+
}
671+
}
672+
673+
log::warn!(
674+
"Could not resolve mDNS service name {} — no matching discovered host",
675+
server
676+
);
677+
server.to_string()
678+
}
679+
680+
/// Extracts the friendly display name from a server address.
681+
///
682+
/// For mDNS service names like `Naspolya._smb._tcp.local`, returns `Naspolya`.
683+
/// For IPs or hostnames, tries `resolve_ip_to_hostname`, falls back to the raw string.
684+
pub(crate) fn friendly_server_name(server: &str) -> String {
685+
// mDNS service name: extract the part before "._"
686+
if server.contains("._tcp") || server.contains("._udp") {
687+
return server.split("._").next().unwrap_or(server).to_string();
688+
}
689+
// IP address: try to resolve to mDNS hostname
690+
resolve_ip_to_hostname(server).unwrap_or_else(|| server.to_string())
691+
}
692+
453693
/// Tries to retrieve SMB credentials from the Keychain.
454694
///
455695
/// Tries multiple keys: by IP (from statfs), by hostname (from mDNS discovery),

‎apps/desktop/src-tauri/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,8 @@ pub fn run() {
861861
#[cfg(any(target_os = "macos", target_os = "linux"))]
862862
commands::network::upgrade_to_smb_volume,
863863
#[cfg(any(target_os = "macos", target_os = "linux"))]
864+
commands::network::upgrade_to_smb_volume_with_credentials,
865+
#[cfg(any(target_os = "macos", target_os = "linux"))]
864866
commands::network::connect_to_server,
865867
#[cfg(any(target_os = "macos", target_os = "linux"))]
866868
commands::network::remove_manual_server,
@@ -903,6 +905,8 @@ pub fn run() {
903905
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
904906
stubs::network::upgrade_to_smb_volume,
905907
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
908+
stubs::network::upgrade_to_smb_volume_with_credentials,
909+
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
906910
stubs::network::connect_to_server,
907911
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
908912
stubs::network::remove_manual_server,

‎apps/desktop/src-tauri/src/network/CLAUDE.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,4 @@ convention) and passes it as an explicit mount point to `NetFSMountURLSync`. The
134134
- **Mount URL must include port when non-standard**: `mount_share_sync` builds `smb://server:port/share` for non-445 ports. The port is passed as a separate parameter through `mount_share` → `mount_share_sync`, not embedded in the server string (embedding it would cause `build_smb_addr` to double the port: `localhost:10480:10480`). `SmbMountInfo.port` extracts the port from `statfs` mount source for upgrade paths.
135135
- **Strip `.local` from addr for smb2**: `smb2::Connection::connect()` extracts `server_name` from the addr string and uses it in UNC paths. Passing `"foo.local:445"` creates `\\foo.local\IPC$` which some servers reject. The `build_addr` helper in `smb_connection.rs` handles this.
136136
- **Manual hosts always set `hostname`**: The share listing pipeline guards on `host.hostname` being truthy. `create_network_host` always sets `hostname` (to the address, even for IPs) so manual hosts flow through the pipeline correctly.
137+
- **`statfs` can return mDNS service names instead of IPs**: When macOS auto-reconnects an SMB mount on login, `statfs.f_mntfromname` may contain `//user@Naspolya._smb._tcp.local/share` instead of `//user@192.168.1.111/share`. These service names are not DNS-resolvable. `resolve_server_address()` in `commands/network.rs` detects these (by checking for `._tcp`/`._udp`) and resolves them to IPs via `get_discovered_hosts()`. All upgrade paths (startup, mount-time, manual) go through this resolution. Similarly, `friendly_server_name()` extracts the display name (e.g., `Naspolya`) for UI display.

‎apps/desktop/src-tauri/src/network/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ pub(crate) mod smb_connection;
3232
mod smb_smbclient;
3333
mod smb_smbutil;
3434
mod smb_types;
35-
mod smb_util;
35+
pub(crate) mod smb_util;
3636

3737
#[cfg(feature = "smb-e2e")]
3838
pub mod virtual_smb_hosts;

0 commit comments

Comments
 (0)