Skip to content

sync: from linuxdeepin/dde-session-shell#512

Open
deepin-ci-robot wants to merge 1 commit into
masterfrom
sync-pr-69-nosync
Open

sync: from linuxdeepin/dde-session-shell#512
deepin-ci-robot wants to merge 1 commit into
masterfrom
sync-pr-69-nosync

Conversation

@deepin-ci-robot

@deepin-ci-robot deepin-ci-robot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#69

Summary by Sourcery

Improve handling of password expiration information during greeter authentication, especially for newly discovered or domain users.

Bug Fixes:

  • Ensure newly discovered users (e.g., domain users on first login) are added to the local model so subsequent authentication can find and update their password expiration state.
  • Avoid attempting to sync password expiration info for accounts not present in the local user model, preventing incorrect or unsafe shadow password reads.

Enhancements:

  • Update the greeter to rely on the user model’s own password expiration update logic instead of directly reading shadow password data via glibc.
  • Refresh SPDX copyright years for greeterworker sources to cover 2022–2026.

@sourcery-ai

sourcery-ai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Synchronizes greeter password-expiration handling and user model updates with upstream dde-session-shell, replacing local shadow-password logic with model-driven updates and improving handling of first-login/domain users.

File-Level Changes

Change Details Files
Refactor password expiration handling to rely on User model methods instead of direct shadow password (getspnam) lookups.
  • Remove GreeterWorker::updatePasswordExpiredStateBySPName helper and its declaration from implementation and header.
  • Replace calls to updatePasswordExpiredStateBySPName with currentUser()->updatePasswordExpiredInfo() when a current user is available.
  • Add null checks around currentUser() before accessing expiredState to avoid potential crashes when no user is selected.
src/lightdm-deepin-greeter/greeterworker.cpp
src/lightdm-deepin-greeter/greeterworker.h
Improve handling of first-login or domain users so they are added to the local user model and log missing user information.
  • When checking an account via AccountsService, add newly discovered users to the local model (m_model->addUser(userPath)) so subsequent authentication can find them.
  • Log a warning instead of attempting to sync password expired info when a user is not found in the local model during createAuthentication.
src/lightdm-deepin-greeter/greeterworker.cpp
Update SPDX copyright header years to cover 2022-2026.
  • Adjust SPDX-FileCopyrightText year range in greeterworker.cpp.
  • Adjust SPDX-FileCopyrightText year range in greeterworker.h.
src/lightdm-deepin-greeter/greeterworker.cpp
src/lightdm-deepin-greeter/greeterworker.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In onAuthFinished, consider storing m_model->currentUser() in a local variable instead of calling it multiple times to make the logic clearer and avoid repeated lookups/null checks.
  • The warning in createAuthentication when the user is not found in the local model might be expected behavior for first-time domain logins; consider using a less severe log level or refining the message to distinguish between normal and error scenarios.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `onAuthFinished`, consider storing `m_model->currentUser()` in a local variable instead of calling it multiple times to make the logic clearer and avoid repeated lookups/null checks.
- The warning in `createAuthentication` when the user is not found in the local model might be expected behavior for first-time domain logins; consider using a less severe log level or refining the message to distinguish between normal and error scenarios.

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.

@deepin-ci-robot

Copy link
Copy Markdown
Contributor Author

deepin pr auto review


★ 总体评分:95分

■ 【总体评价】

代码成功移除了不安全的底层接口调用并修复了空指针解引用风险,整体质量优秀
逻辑完全正确且消除了安全隐患,因存在轻微的重复函数调用扣5分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

修改精准解决了域管账户首次登录时因后端信息未就绪导致获取不到用户对象的问题,通过在 checkAccount 中提前调用 m_model->addUser 将用户纳入本地模型,确保后续 createAuthenticationfindUserByName 能正确找到用户对象,逻辑闭环完整。
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

删除了冗余且职责不清晰的 updatePasswordExpiredStateBySPName 函数,增加了必要的空指针保护和清晰的注释说明,代码结构更加清晰。
潜在问题:onAuthFinished 函数中连续两次调用了 m_model->currentUser(),存在轻微的代码冗余。
建议:将 m_model->currentUser() 的返回值缓存到局部变量中,提高代码可读性并减少不必要的函数调用开销。

  • 3.代码性能(高效)✓

移除了对 getspnam 的系统调用,避免了直接读取 /etc/shadow 文件带来的 I/O 开销,性能得到实际提升。
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改有效消除了原有代码中直接依赖 glibc 底层接口读取敏感文件的风险,并修复了潜在的空指针解引用导致的本地拒绝服务漏洞,未引入任何新的安全风险。

  • 建议:继续保持避免在应用层直接调用底层密码接口的安全编码实践。

■ 【改进建议代码示例】

void GreeterWorker::onAuthFinished()
{
    qCInfo(DDE_SHELL) << "Auth finished";
    if (m_greeter->inAuthentication()) {
        m_greeter->respond(m_authFramework->AuthSessionPath(m_account) + QString(";") + m_password);
        
        // 缓存用户指针,避免多次调用 m_model->currentUser() 并集中进行空指针校验
        User *currentUser = m_model->currentUser();
        if (currentUser) {
            currentUser->updatePasswordExpiredInfo();
            if (currentUser->expiredState() == User::ExpiredAlready) {
                changePasswd();
            }
        }
    } else {
        // ... 后续逻辑保持不变
    }
}

@deepin-ci-robot

Copy link
Copy Markdown
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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

Details 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

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#69
@deepin-bot

deepin-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 6.0.62
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #514

@deepin-bot

deepin-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 6.0.63
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #516

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.

1 participant