Skip to content

Conversation

@pengfeixx
Copy link
Contributor

@pengfeixx pengfeixx commented Sep 8, 2025

Fix the biometric authentication refresh not being timely.

Log: Fix the biometric authentication refresh not being timely.
pms: BUG-332419

Summary by Sourcery

Improve biometric authentication refresh to update driver status and fingerprint enrollment list immediately.

Bug Fixes:

  • Invoke refreshFingerDriverStatus after retrieving the default device to ensure timely update of biometric status
  • Implement refreshFingerDriverStatus slot to validate device, update model state, and refresh or clear fingerprint enrollment list

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pengfeixx

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

Needs approval from an approver in each of these files:

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

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 8, 2025

Reviewer's Guide

This PR ensures the biometric authentication status is refreshed immediately by invoking a new handler during driver info retrieval and introducing a dedicated method to update the model state and enrollment list based on the default device availability.

Sequence diagram for biometric authentication refresh on driver info retrieval

sequenceDiagram
    participant CharaMangerWorker
    participant CharaMangerInter
    participant CharaMangerModel
    participant SystemUser
    CharaMangerWorker->>CharaMangerInter: driverInfo()
    CharaMangerWorker->>CharaMangerInter: defaultDevice()
    CharaMangerWorker->>CharaMangerWorker: refreshFingerDriverStatus(defaultDevice)
    alt defaultDevice is valid
        CharaMangerWorker->>SystemUser: getuid() / getpwuid()
        CharaMangerWorker->>CharaMangerWorker: refreshFingerEnrollList(userId)
    else defaultDevice is not valid
        CharaMangerWorker->>CharaMangerModel: setThumbsList([])
    end
    CharaMangerWorker->>CharaMangerModel: setFingerVaild(isFingerValid)
Loading

Class diagram for updated CharaMangerWorker methods

classDiagram
    class CharaMangerWorker {
        +void refreshDriverInfo()
        +void refreshFingerDriverStatus(const QString &defaultDevice)
        -CharaMangerModel *m_model
    }
    class CharaMangerModel {
        +void setFingerVaild(bool)
        +void setThumbsList(QStringList)
    }
    CharaMangerWorker --> CharaMangerModel
Loading

File-Level Changes

Change Details Files
Invoke the new status refresh in driver info flow
  • Retrieve the defaultDevice from the interface
  • Add call to refreshFingerDriverStatus(defaultDevice)
src/plugin-authentication/operation/charamangerworker.cpp
Implement and expose refreshFingerDriverStatus to update model state
  • Declare the refreshFingerDriverStatus slot in the header
  • Define logic to set finger validity and refresh or clear enroll list
  • Obtain current user via getpwuid when a device is valid
src/plugin-authentication/operation/charamangerworker.cpp
src/plugin-authentication/operation/charamangerworker.h

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • Add a null check for getpwuid(getuid()) before dereferencing pws to avoid potential crashes if the user lookup fails.
  • Consider extracting the user-name retrieval into a separate helper or injecting it, so refreshFingerDriverStatus only handles driver state logic and not system calls.
  • There’s a typo in the model setter and usage: setFingerVaild should be setFingerValid for consistency and readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a null check for getpwuid(getuid()) before dereferencing pws to avoid potential crashes if the user lookup fails.
- Consider extracting the user-name retrieval into a separate helper or injecting it, so refreshFingerDriverStatus only handles driver state logic and not system calls.
- There’s a typo in the model setter and usage: `setFingerVaild` should be `setFingerValid` for consistency and readability.

## Individual Comments

### Comment 1
<location> `src/plugin-authentication/operation/charamangerworker.cpp:427` </location>
<code_context>
+    if (isFingerValid) {
+        struct passwd *pws;
+        QString userId;
+        pws = getpwuid(getuid());
+        userId = QString(pws->pw_name);
+        refreshFingerEnrollList(userId);
+    } else {
</code_context>

<issue_to_address>
Potential null pointer dereference with getpwuid.

Add a null check for pws before accessing pw_name to prevent crashes if getpwuid returns nullptr.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +427 to +428
pws = getpwuid(getuid());
userId = QString(pws->pw_name);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Potential null pointer dereference with getpwuid.

Add a null check for pws before accessing pw_name to prevent crashes if getpwuid returns nullptr.

Fix the biometric authentication refresh not being timely.

Log: Fix the biometric authentication refresh not being timely.
pms: BUG-332419
@deepin-ci-robot
Copy link

deepin pr auto review

我对这段代码进行了审查,发现了一些可以改进的地方:

  1. 代码安全方面:
  • refreshFingerDriverStatus 函数中使用了 getpwuid(getuid()) 来获取用户信息,但没有检查返回值是否为 NULL,这可能导致空指针解引用的风险。应该添加检查:
if (pws == nullptr) {
    qWarning() << "Failed to get user information";
    return;
}
  1. 代码性能方面:
  • refreshFingerDriverStatus 函数中每次调用都会调用 getpwuid(getuid()),但实际上这个值在程序运行期间是不会变的,可以考虑在初始化时缓存这个值,避免重复系统调用。
  1. 代码质量方面:
  • 函数命名不一致:setFingerVaild 中的 "Vaild" 是拼写错误,应该是 "Valid"。建议修正这个拼写错误,保持命名一致性。
  • refreshFingerDriverStatus 函数中的 QString(userId) 转换是多余的,因为 pws->pw_name 已经是 char* 类型,可以直接用于 QString 构造。
  1. 代码逻辑方面:
  • refreshDriverInfo 函数中调用 refreshFingerDriverStatus 时,没有对返回的 defaultDevice 进行有效性检查就直接传递了。虽然 refreshFingerDriverStatus 内部有检查,但在调用前进行检查会更符合防御性编程的原则。

改进建议:

void CharaMangerWorker::refreshDriverInfo()
{
    auto driverInfo = m_charaMangerInter->driverInfo();
    predefineDriverInfo(driverInfo);
    
    auto defaultDevice = m_charaMangerInter->defaultDevice();
    if (!defaultDevice.isEmpty()) {
        refreshFingerDriverStatus(defaultDevice);
    }
}

void CharaMangerWorker::refreshFingerDriverStatus(const QString &defaultDevice)
{
    bool isFingerValid = !defaultDevice.isEmpty();
    m_model->setFingerValid(isFingerValid);  // 修正拼写错误
    
    if (isFingerValid) {
        struct passwd *pws = getpwuid(getuid());
        if (pws == nullptr) {
            qWarning() << "Failed to get user information";
            return;
        }
        
        refreshFingerEnrollList(pws->pw_name);  // 直接使用 char*,避免不必要的转换
    } else {
        m_model->setThumbsList(QStringList());
    }
}
  1. 其他建议:
  • 考虑在类的私有成员中添加 m_currentUserId 来缓存用户ID,避免重复调用 getpwuid
  • 可以考虑添加日志记录,便于调试和追踪问题。

这些改进将提高代码的安全性、性能和可维护性。

@deepin-bot
Copy link

deepin-bot bot commented Sep 11, 2025

TAG Bot

New tag: 6.1.47
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2667

@deepin-bot
Copy link

deepin-bot bot commented Sep 18, 2025

TAG Bot

New tag: 6.1.48
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2689

@deepin-bot
Copy link

deepin-bot bot commented Sep 25, 2025

TAG Bot

New tag: 6.1.49
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2708

@deepin-bot
Copy link

deepin-bot bot commented Sep 26, 2025

TAG Bot

New tag: 6.1.50
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2709

@deepin-bot
Copy link

deepin-bot bot commented Oct 10, 2025

TAG Bot

New tag: 6.1.51
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2724

@deepin-bot
Copy link

deepin-bot bot commented Oct 16, 2025

TAG Bot

New tag: 6.1.52
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants