Url encode username in sftp connection string #1731
Conversation
📝 WalkthroughWalkthroughSFTP URL generation in server settings now percent-encodes the authenticated username. SFTP authentication lookup normalizes input by lowercasing and trimming. Stored user username and email are normalized to lowercase and trimmed via the string helper. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Filament Settings UI
participant Auth as Auth
participant Controller as SftpAuthenticationController
participant DB as User Model / DB
UI->>Auth: request current user
Auth-->>UI: returns user.username
UI->>UI: rawurlencode(username) -> build sftp://{encoded}.{uuid_short}@{fqdn}:{port}
Note right of UI #DFF2E1: Encoded username prevents\ninvalid URL characters
UI->>Controller: POST /api/remote/sftp-auth { username: userInput }
Controller->>Controller: normalize input (lowercase + trim)
Controller->>DB: query user by normalized username
alt user found
DB-->>Controller: user record
Controller-->>UI: 200 OK (auth result)
else not found
DB-->>Controller: null
Controller-->>UI: 4xx / reject
end
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/Filament/Server/Pages/Settings.php (2)
191-195: Duplicate label() call on password entry.Two label() calls: the latter overrides the first. If the second is meant as helper/hint text, switch to the appropriate API; otherwise remove the duplicate.
175-178: Deduplicate SFTP URI construction and bracket IPv6 hostsConfirmed duplicate SFTP URI concatenation in app/Filament/Server/Pages/Settings.php (connect_sftp->url closure and formatStateUsing closure). Replace both with a helper that rawurlencodes the username and brackets IPv6 literals:
private function buildSftpUri(Server $server): string { $user = rawurlencode(auth()->user()->username) . '.' . $server->uuid_short; $host = $server->node->daemon_sftp_alias ?? $server->node->fqdn; if (strpos($host, ':') !== false && ($host[0] ?? '') !== '[') { $host = '[' . trim($host, '[]') . ']'; } return sprintf('sftp://%s@%s:%d', $user, $host, (int) $server->node->daemon_sftp); }Wire both closures to call this helper and remove the duplicated concatenations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Server/Pages/Settings.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Server/Pages/Settings.php (2)
app/Models/Server.php (3)
user(259-262)node(331-334)Server(132-504)app/Models/User.php (1)
username(219-224)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/Models/User.php (1)
195-195: Remove redundant email normalization in saving hook.Since Line 229 now handles email normalization through the
email()mutator, the explicit normalization in thesavinghook is redundant and could lead to double processing.static::saving(function (self $user) { - $user->email = mb_strtolower($user->email); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Http/Controllers/Api/Remote/SftpAuthenticationController.php(1 hunks)app/Models/User.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Http/Controllers/Api/Remote/SftpAuthenticationController.php (1)
app/Models/User.php (2)
User(95-493)username(219-224)
🔇 Additional comments (4)
app/Models/User.php (2)
222-222: LGTM: Username normalization aligns with SFTP authentication.The change to normalize usernames using
str($value)->lower()->trim()->toString()is correct and ensures consistency with the SFTP authentication lookup inSftpAuthenticationController.php.
229-229: LGTM: Email normalization follows same pattern as username.The email normalization using the fluent string helper is consistent with the username normalization and maintains proper data consistency.
app/Http/Controllers/Api/Remote/SftpAuthenticationController.php (2)
102-102: LGTM: Username normalization ensures consistent authentication.The normalization using
str($username)->lower()->trim()correctly aligns with the User model's username mutator, ensuring that SFTP authentication works consistently regardless of how the username was originally stored or provided.
100-105: Verify SFTP authentication handles special characters in usernames
- Found: Settings UI builds sftp:// URLs with rawurlencode(auth()->user()->username) — app/Filament/Server/Pages/Settings.php (lines ~177,183).
- Action: getUser() (app/Http/Controllers/Api/Remote/SftpAuthenticationController.php, ~lines 100–105) only lowercases/trims the username and does not rawurldecode. Confirm whether the incoming username is URL-decoded by the SFTP client/daemon; if not, decode before lookup (e.g. $username = rawurldecode($username); then str($username)->lower()->trim()).
Closes #1726