Fix: Can not open deepin-devicemanager.

[GongHeng2017]

--Polkit daemon (polkitd) runs on the system bus and validates DBus clients based on system bus connection names. Passing a session bus connection name via SystemBusNameSubject(QDBusConnection::sessionBus().baseService()) was invalid causing Polkit to fail the credential lookups, which resulted in Authority::Unknown and silent authorization failures.

--This commit fixes the issue by passing the correct system bus connection name (QDBusConnection::systemBus().baseService()) to the Polkit authorization subject, ensuring the credentials can be correctly verified and the policy authentication prompts will work as expected.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-352627.html

Summary by Sourcery

Bug Fixes:

• Ensure Polkit credential lookups succeed by passing the system bus connection name instead of the session bus name during authorization checks for application startup and driver installation.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR fixes Polkit authorization failures in deepin-devicemanager by using the correct system D-Bus connection name when constructing the SystemBusNameSubject for authorization checks, both at startup and before driver installation.

Sequence diagram for Polkit authorization at application startup

sequenceDiagram
    actor User
    participant DeepinDeviceManager as DeepinDeviceManager
    participant QDBusSystemBus as QDBusSystemBus
    participant PolkitAuthority as PolkitAuthority
    participant polkitd as polkitd

    User->>DeepinDeviceManager: startApplication()
    DeepinDeviceManager->>QDBusSystemBus: baseService()
    QDBusSystemBus-->>DeepinDeviceManager: systemBusName

    DeepinDeviceManager->>PolkitAuthority: checkAuthorizationSync(checkAuthentication, SystemBusNameSubject(systemBusName), AllowUserInteraction)

    PolkitAuthority->>polkitd: CheckAuthorization(SystemBusNameSubject(systemBusName))
    polkitd-->>PolkitAuthority: Result Yes or No
    PolkitAuthority-->>DeepinDeviceManager: Result Yes or No

    alt Authorization Yes
        DeepinDeviceManager-->>User: showMainWindow()
    else Authorization Not Yes
        DeepinDeviceManager-->>User: exitApplication()
    end

Loading

Sequence diagram for Polkit authorization before driver installation

sequenceDiagram
    actor User
    participant DeepinDeviceManager as DeepinDeviceManager
    participant QDBusSystemBus as QDBusSystemBus
    participant PolkitAuthority as PolkitAuthority
    participant polkitd as polkitd

    User->>DeepinDeviceManager: installDriverLogical()
    DeepinDeviceManager->>DeepinDeviceManager: evaluateCurrentIndex()
    alt DriverInstallRequiresPrivilege
        DeepinDeviceManager->>QDBusSystemBus: baseService()
        QDBusSystemBus-->>DeepinDeviceManager: systemBusName

        DeepinDeviceManager->>PolkitAuthority: checkAuthorizationSync(checkAuthentication, SystemBusNameSubject(systemBusName), AllowUserInteraction)

        PolkitAuthority->>polkitd: CheckAuthorization(SystemBusNameSubject(systemBusName))
        polkitd-->>PolkitAuthority: Result Yes or No
        PolkitAuthority-->>DeepinDeviceManager: Result Yes or No

        alt Authorization Yes
            DeepinDeviceManager->>DeepinDeviceManager: performDriverInstallation()
            DeepinDeviceManager-->>User: showInstallationResult()
        else Authorization Not Yes
            DeepinDeviceManager-->>User: cancelInstallation()
        end
    else OtherIndexPath
        DeepinDeviceManager->>DeepinDeviceManager: executeOtherLogic()
    end

Loading

File-Level Changes

Change	Details	Files
Use the system D-Bus connection name instead of the session bus name for Polkit authorization subject creation.	Change the Polkit authorization subject in the driver installation flow to use QDBusConnection::systemBus().baseService() when constructing SystemBusNameSubject Change the Polkit authorization subject in application startup authorization to use QDBusConnection::systemBus().baseService() when constructing SystemBusNameSubject	deepin-devicemanager/src/Page/PageDriverControl.cpp deepin-devicemanager/src/main.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The authorization subject construction now appears in both main.cpp and PageDriverControl.cpp; consider refactoring this into a shared helper to avoid duplication and ensure future changes to the bus/subject stay consistent.
• When authorization fails (result != Authority::Yes), the code returns silently; consider adding minimal logging or user feedback so failures are diagnosable instead of failing without visible reason.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The authorization subject construction now appears in both main.cpp and PageDriverControl.cpp; consider refactoring this into a shared helper to avoid duplication and ensure future changes to the bus/subject stay consistent.
- When authorization fails (result != Authority::Yes), the code returns silently; consider adding minimal logging or user feedback so failures are diagnosable instead of failing without visible reason.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

deepin pr auto review

这段代码的修改主要涉及将 D-Bus 连接从 sessionBus（会话总线）更改为 systemBus（系统总线）。以下是对此修改的详细审查和改进意见：

1. 语法逻辑审查

修改内容：
将 QDBusConnection::sessionBus() 修改为 QDBusConnection::systemBus()。

逻辑分析：

• 原代码：使用 sessionBus（会话总线）。会话总线通常用于用户会话内的进程间通信，权限较低，且与特定用户会话绑定。
• 修改后：使用 systemBus（系统总线）。系统总线用于系统级服务通信，通常需要更高权限，且不依赖特定用户会话。

判断：

• 如果 com.deepin.deepin-devicemanager.checkAuthentication 是一个需要在系统级别验证权限的操作（例如安装驱动、修改系统设置），则修改为 systemBus 是合理的。
• 如果该操作仅限于当前用户会话，则应保持 sessionBus。

结论：根据代码上下文（安装驱动、提权），修改为 systemBus 是正确的，因为驱动安装通常需要系统级权限。

---

2. 代码质量审查

问题：

1. 硬编码字符串："com.deepin.deepin-devicemanager.checkAuthentication" 是硬编码的，容易出错且难以维护。
2. 重复代码：两处修改（PageDriverControl.cpp 和 main.cpp）都包含相同的权限检查逻辑，建议封装为函数。

改进建议：

1. 将权限检查逻辑封装为独立函数，避免重复代码。
2. 将硬编码字符串定义为常量。

改进后的代码示例：

// 在头文件中定义常量
constexpr const char* AUTH_ACTION = "com.deepin.deepin-devicemanager.checkAuthentication";

// 封装权限检查函数
bool checkSystemAuthorization() {
    Authority::Result result = Authority::instance()->checkAuthorizationSync(
        AUTH_ACTION,
        SystemBusNameSubject(QDBusConnection::systemBus().baseService()),
        Authority::AllowUserInteraction
    );
    return result == Authority::Yes;
}

// 使用示例
void PageDriverControl::installDriverLogical() {
    if (!checkSystemAuthorization()) {
        return;
    }
    // 其他逻辑...
}

---

3. 代码性能审查

问题：

• checkAuthorizationSync 是同步调用，会阻塞当前线程。如果权限验证耗时较长（例如网络延迟或用户输入密码），会导致界面冻结。

改进建议：

1. 使用异步版本 checkAuthorization，避免阻塞主线程。
2. 如果必须使用同步版本，建议在后台线程中执行。

改进后的代码示例（异步版本）：

void PageDriverControl::installDriverLogical() {
    Authority::instance()->checkAuthorization(
        AUTH_ACTION,
        SystemBusNameSubject(QDBusConnection::systemBus().baseService()),
        Authority::AllowUserInteraction,
        [](Authority::Result result) {
            if (result == Authority::Yes) {
                // 权限验证通过，执行后续逻辑
            } else {
                // 权限验证失败
            }
        }
    );
}

---

4. 代码安全审查

问题：

1. 权限验证失败处理：当前代码在权限验证失败时直接返回，未提示用户具体原因。
2. D-Bus 安全性：systemBus 的权限更高，需确保服务端（com.deepin.deepin-devicemanager）已正确配置 Polkit 策略，防止未授权访问。

改进建议：

1. 在权限验证失败时，向用户显示明确的错误信息。
2. 确保 Polkit 策略文件（如 /usr/share/polkit-1/actions/com.deepin.deepin-devicemanager.policy）已正确配置。

改进后的代码示例：

void PageDriverControl::installDriverLogical() {
    if (!checkSystemAuthorization()) {
        QMessageBox::warning(this, tr("Error"), tr("Permission denied. Please try again with administrator privileges."));
        return;
    }
    // 其他逻辑...
}

---

总结

1. 语法逻辑：修改为 systemBus 是正确的，符合驱动安装等系统级操作的需求。
2. 代码质量：建议封装权限检查逻辑，避免硬编码字符串。
3. 代码性能：建议使用异步权限检查，避免阻塞主线程。
4. 代码安全：需完善错误提示，并确保 Polkit 策略正确配置。

以上改进建议可提升代码的可维护性、性能和安全性。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, KT-lcz, max-lvs

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[GongHeng2017]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)