fix(network): pre-fill saved VPN password from the GetSecrets payload - #1472
fix(network): pre-fill saved VPN password from the GetSecrets payload#1472lkramer wants to merge 1 commit into
Conversation
|
Thank you for the contribution. #1453 is already in progress to update nmrs to version 3.4, while also migrating some other plumbing to nmrs. This PR also updates nmrs to version 3.4. Would it be reasonable for us to wait to review this until that one's finished? |
ah, I hadn't spotted that. I will rebase this one once #1453 is merged. |
| } | ||
|
|
||
| /// Pick a VPN secret from NM's payload: a hinted key first, else `"password"`. | ||
| fn pick_secret(src: &HashMap<String, String>, keys: &[String]) -> Option<String> { |
There was a problem hiding this comment.
Small nit pick if you don't mind: if a hinted key exists but its value is empty, this returns that empty value and filters to None without trying the "password" fallback. Could this skip a valid saved password when NM includes an empty hinted secret plus a populated fallback key?
Maybe filter empties before selecting:
keys.iter()
.filter_map(|k| src.get(k).filter(|s| !s.is_empty()))
.next()
.or_else(|| src.get("password").filter(|s| !s.is_empty()))
.cloned()There was a problem hiding this comment.
Ah, nice spot! I updated my code.
I don't know what the convention is, the code seems lightweight on unit tests, but to me it made sense to add some for this function to confirm your edge case was solved, so I did. Let me know if you want me to remove them.
`cosmic-applet-network` opened the VPN secret prompt with an empty password field even when NetworkManager already had the password saved, because the nmrs agent request discarded the secrets NM includes in its `GetSecrets` payload. nmrs 3.4.0 surfaces them as `SecretRequest.existing_secrets`, so the applet pre-fills the dialog from that map — a hinted key, else `"password"` — at the `RequestSecret` site. Only system-owned secrets appear in the payload (agent-owned and not-saved secrets are never sent to agents), so this covers the common "saved for all users" case with no keyring handling. - Built and deployed against a real OpenVPN connection (`yoti_vpn`, `password-flags=0`) on Pop!_OS. Before: the field opened blank. After: the saved password is pre-filled on the first prompt.
674c0fa to
2acc78b
Compare
3 participants
cosmic-applet-networkopened the VPN secret prompt with an empty password field even when NetworkManager already had the password saved, because the nmrs agent request discarded the secrets NM includes in itsGetSecretspayload.nmrs 3.4.0 ( freedesktop-rs/nmrs#456 ) surfaces them as
SecretRequest.existing_secrets, so the applet pre-fills the dialog from that map, a hinted key, else"password", at theRequestSecretsite. Only system-owned secrets appear in the payload (agent-owned and not-saved secrets are never sent to agents), so this covers the common "saved for all users" case with no keyring handling.yoti_vpn,password-flags=0) on Pop!_OS. Before: the field opened blank. After: the saved password is pre-filled on the first prompt.Note: claude Opus 4.8 was used for coding assistance.
This closes #1444