fix: SMB共享凭证加入share name属性隔离 - #384
Conversation
1. Root cause: DNetworkMounter's keyring credential management
(savePasswd/loginPasswd/smbSchema) did not include share name,
so loginPasswd searched by {server, protocol} only and returned
all saved credentials for the same server regardless of share
2. Fix: add "object" attribute to smbSchema for share name, extract
share from URL path in savePasswd and loginPasswd, include it in
both store and search operations to isolate credentials per share
3. Impact: credentials are now stored and searched per-share; old
credentials without "object" attribute won't be matched, users
need to re-enter password once per share after upgrade
4. 方案层级: 根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)
Log: fix SMB share only authenticating once across different users
Influence:
1. Test accessing non-anonymous shares of different users on the
same server, second share should prompt for authentication
2. Test re-accessing a saved share, credential should auto-fill
3. Test anonymous share access is unaffected
4. Test FTP/WebDAV mounting is unaffected
fix: SMB共享凭证加入share name属性隔离
1. 根因:DNetworkMounter 的 keyring 凭证管理(savePasswd/loginPasswd/
smbSchema)未包含 share name 属性,loginPasswd 仅按
{server, protocol} 搜索,返回同一服务器所有共享的已保存凭证
2. 方案:在 smbSchema 中新增 "object" 属性用于 share name,
savePasswd 和 loginPasswd 从 URL 提取 share name 并在存储和
查询时附带,实现凭证按共享级别隔离
3. 影响:凭证按共享级别存储和搜索,旧凭证不含 "object" 属性
无法命中,升级后用户需对每个已保存共享重新输入一次密码
4. 方案层级:根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)
Log: 修复访问不同用户不同密码的SMB共享时仅鉴权一次的问题
Influence:
1. 测试同一服务器不同用户的非匿名共享,访问第二个共享应弹出鉴权窗口
2. 测试重复访问已保存的共享,凭证应自动填充无需再次输入
3. 测试匿名共享访问不受影响
4. 测试 FTP/WebDAV 挂载不受影响
PMS: BUG-375291
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes SMB credential isolation by deriving the share name from the URL and adding it as a Secret Service schema attribute used consistently when saving and retrieving credentials. Review the URL path parsing and compatibility implications: legacy entries without Sequence diagram for per-share SMB credential lookup and storagesequenceDiagram
participant Mounter as DNetworkMounter
participant SecretService
Mounter->>Mounter: QUrl::path()
Mounter->>SecretService: secret_password_lookup_sync(server, protocol, object)
alt matching share credential exists
SecretService-->>Mounter: saved password
else no matching share credential
SecretService-->>Mounter: no credential
Mounter->>SecretService: secret_password_store_sync(server, protocol, user, object)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dfm-mount/private/dnetworkmounter.cpp" line_range="118" />
<code_context>
QUrl u(address);
QString protocol = u.scheme();
QString host = u.host();
+ QString share = u.path().remove("/");
GHashTable_autoptr query = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
</code_context>
<issue_to_address>
**issue (bug_risk):** `u.path().remove("/")` removes every slash instead of extracting the SMB share component. For an address such as `smb://server/share/subdirectory`, the credential is keyed as `sharesubdirectory`, so it does not match the credential saved or queried for the actual share `share`; credentials therefore fail to auto-fill for valid SMB URLs containing a subpath, and distinct paths can be collapsed into the same key.
**Triggers:** When an SMB address contains a path below the share root, such as `smb://server/share/subdirectory`.
**Suggested fix:** Extract only the first non-empty path component as the share name, while preserving the rest of the URL path for mounting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QUrl u(address); | ||
| QString protocol = u.scheme(); | ||
| QString host = u.host(); | ||
| QString share = u.path().remove("/"); |
There was a problem hiding this comment.
issue (bug_risk): u.path().remove("/") removes every slash instead of extracting the SMB share component. For an address such as smb://server/share/subdirectory, the credential is keyed as sharesubdirectory, so it does not match the credential saved or queried for the actual share share; credentials therefore fail to auto-fill for valid SMB URLs containing a subpath, and distinct paths can be collapsed into the same key.
Triggers: When an SMB address contains a path below the share root, such as smb://server/share/subdirectory.
Suggested fix: Extract only the first non-empty path component as the share name, while preserving the rest of the URL path for mounting.
deepin pr auto reviewAI 代码审查报告总体评分:100分(通过阈值: 70分) 审查结果:通过 总体评价
语法逻辑 ✓ 语法正确,逻辑清晰(25/25分)本次修改目的明确:在 libsecret keyring 的 SMB 凭证 schema 中新增 "object" 属性(对应 share name),并在凭证的存储和查询流程中一致地使用该属性。 代码变更分析: 1.第46行新增常量 未发现语法错误、逻辑缺陷或边界处理问题。 代码质量 ✓ 代码结构清晰,注释完整(25/25分)1.新增代码完全遵循现有代码模式:常量命名使用 代码性能 ✓ 性能良好,资源使用合理(20/20分)1.每次调用仅新增一次 代码安全 ✓ 存在0个安全漏洞(30/30分)存在0个安全漏洞 漏洞对比统计:新增漏洞0个,减少漏洞0个,持平0个 安全分析: 1.本次修改是一项安全改进:修复前,同一服务器上不同 SMB 共享的凭证会被错误复用(因 keyring 查询仅按 server + protocol 匹配),导致用户 A 的共享凭证可能被用于访问用户 B 的共享。修复后,凭证按 {server, protocol, user, domain, object(share name)} 五元组隔离 未发现安全漏洞。OCR 审查因网络问题未能执行代码拉取,使用空结果合并。 本报告由 AI 代码审查工具自动生成 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
fix: include share name in SMB keyring credentials
(savePasswd/loginPasswd/smbSchema) did not include share name,
so loginPasswd searched by {server, protocol} only and returned
all saved credentials for the same server regardless of share
share from URL path in savePasswd and loginPasswd, include it in
both store and search operations to isolate credentials per share
credentials without "object" attribute won't be matched, users
need to re-enter password once per share after upgrade
Log: fix SMB share only authenticating once across different users
Influence:
same server, second share should prompt for authentication
fix: SMB共享凭证加入share name属性隔离
smbSchema)未包含 share name 属性,loginPasswd 仅按
{server, protocol} 搜索,返回同一服务器所有共享的已保存凭证
savePasswd 和 loginPasswd 从 URL 提取 share name 并在存储和
查询时附带,实现凭证按共享级别隔离
无法命中,升级后用户需对每个已保存共享重新输入一次密码
Log: 修复访问不同用户不同密码的SMB共享时仅鉴权一次的问题
Influence:
PMS: BUG-375291
Summary by Sourcery
Scope SMB credential storage and lookup to the share name to ensure each share is authenticated with its own credentials.
Bug Fixes:
Enhancements: