Skip to content

fix: remove glibc-based password expiry check and fix domain user#69

Merged
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master
Jul 6, 2026
Merged

fix: remove glibc-based password expiry check and fix domain user#69
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin

@xionglinlin xionglinlin commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

handling

  1. Remove updatePasswordExpiredStateBySPName() function that used glibc getspnam to check password expiry, as it was only needed for first-time domain user login
  2. Ensure new domain users discovered in checkAccount() are added to the local model immediately, so subsequent operations like createAuthentication() can find them via findUserByName()
  3. Fall back to using updatePasswordExpiredInfo() through existing model interfaces for all users, including newly discovered ones
  4. Fix a crash in onAuthFinished() where m_model->currentUser() could be null when checking expiredState()

Log: Fix domain user first login; remove redundant glibc password expiry logic

Influence:

  1. Test domain user first login scenario to verify password expiry info is correctly synced
  2. Test normal user login to ensure no regression with password expiry checks
  3. Verify no crashes occur during authentication finish for any user type
  4. Test multi-user switching, including between local and domain users

fix: 移除基于 glibc 的密码过期检查并修正域管用户处理

  1. 移除 updatePasswordExpiredStateBySPName() 函数,该函数使用 glibc 的 getspnam 检查密码过期,只在域管用户首次登录时需要
  2. 确保在 checkAccount() 中新发现的域管用户立即加入本地 model,使得后 续 createAuthentication() 等操作能通过 findUserByName() 找到
  3. 所有用户(包括新发现用户)都回退到通过现有 model 接口使用 updatePasswordExpiredInfo()
  4. 修复 onAuthFinished()m_model->currentUser() 可能为空时检查 expiredState() 导致的崩溃

Log: 修复域管用户首次登录;移除冗余的 glibc 密码过期逻辑

Influence:

  1. 测试域管用户首次登录场景,验证密码过期信息是否正确同步
  2. 测试普通用户登录,确保密码过期检查无回归
  3. 验证任意用户在认证结束时不会发生崩溃
  4. 测试多用户切换,包括本地用户和域管用户之间的切换

PMS: BUG-367143
Change-Id: Icbbe23ae6246d4e60c1dbc34d35a4f414d86e8df

Summary by Sourcery

Align password expiry handling with the user model and remove legacy glibc-based logic.

Bug Fixes:

  • Ensure newly discovered domain users are added to the local model so authentication and password expiry checks work on first login.
  • Prevent a crash in authentication completion when the current user is null and password expiry state is accessed.

Enhancements:

  • Rely consistently on the existing user model’s password expiry update mechanism instead of direct glibc calls.
  • Log a warning when authentication is created for an account that does not yet exist in the local user model.

Chores:

  • Update SPDX copyright years for the greeter worker implementation and header.

@sourcery-ai

sourcery-ai Bot commented Jun 29, 2026

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

Reviewer's Guide

Removes the custom glibc-based password expiry helper and relies solely on the model’s updatePasswordExpiredInfo(), while ensuring newly discovered domain users are added to the local model and fixing a crash when currentUser() is null during auth completion.

File-Level Changes

Change Details Files
Rely exclusively on model-driven password expiry updates instead of glibc getspnam-based logic.
  • Removed the fallback path in createAuthentication() that called updatePasswordExpiredStateBySPName() when a user was not found in the local model.
  • Added a warning log in createAuthentication() when the user is missing from the model and password expiry info cannot be synced.
  • Deleted the updatePasswordExpiredStateBySPName() method implementation and its declaration from the class.
src/lightdm-deepin-greeter/greeterworker.cpp
src/lightdm-deepin-greeter/greeterworker.h
Ensure newly discovered users (e.g., domain users on first login) are immediately added to the local user model.
  • In checkAccount(), add the user to the model via addUser() when a user is discovered but originalUsePtr is null, so subsequent logic can findUserByName().
src/lightdm-deepin-greeter/greeterworker.cpp
Prevent a crash during authentication finish when currentUser() can be null and keep password expiry info in sync via model APIs.
  • In onAuthFinished(), replace the call to updatePasswordExpiredStateBySPName() with currentUser()->updatePasswordExpiredInfo() guarded by a null check.
  • Add defensive checks before accessing currentUser()->expiredState() to avoid dereferencing a null pointer and still invoke changePasswd() when the user is expired.
src/lightdm-deepin-greeter/greeterworker.cpp
Update SPDX file copyright headers to include the extended year range.
  • Extend SPDX-FileCopyrightText years from 2022 to 2022 - 2026 in the source and header files.
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 found 1 issue, and left some high level feedback:

  • In createAuthentication, instead of just logging that password expiry info will not be synced when the user is missing from the model, consider explicitly documenting or handling how/when that expiry info will be synchronized later to avoid silent divergence from backend state.
  • In onAuthFinished, you can simplify and slightly optimize the repeated m_model->currentUser() lookup by storing it in a local variable before the null checks and reuse that pointer for both updating expiry info and checking expiredState().
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `createAuthentication`, instead of just logging that password expiry info will not be synced when the user is missing from the model, consider explicitly documenting or handling how/when that expiry info will be synchronized later to avoid silent divergence from backend state.
- In `onAuthFinished`, you can simplify and slightly optimize the repeated `m_model->currentUser()` lookup by storing it in a local variable before the null checks and reuse that pointer for both updating expiry info and checking `expiredState()`.

## Individual Comments

### Comment 1
<location path="src/lightdm-deepin-greeter/greeterworker.cpp" line_range="890-893" />
<code_context>
         m_greeter->respond(m_authFramework->AuthSessionPath(m_account) + QString(";") + m_password);
-        updatePasswordExpiredStateBySPName(m_account);
-        if (m_model->currentUser()->expiredState() == User::ExpiredAlready) {
+        if (m_model->currentUser()) {
+            m_model->currentUser()->updatePasswordExpiredInfo();
+        }
+        if (m_model->currentUser() && m_model->currentUser()->expiredState() == User::ExpiredAlready) {
             changePasswd();
         }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider avoiding repeated `currentUser()` lookups and ensure it refers to the authenticating account.

In `onAuthFinished`, `m_model->currentUser()` is invoked three times and implicitly assumed to match `m_account`. Please capture it once (e.g. `auto user = m_model->currentUser();`) and use that pointer. This reduces redundant lookups, minimizes race-condition risk, and makes it explicit that you’re operating on the user associated with the completed auth session.

Suggested implementation:

```cpp
        m_greeter->respond(m_authFramework->AuthSessionPath(m_account) + QString(";") + m_password);

        auto user = m_model->currentUser();
        if (user) {
            user->updatePasswordExpiredInfo();

            if (user->expiredState() == User::ExpiredAlready) {
                changePasswd();
            }
        }

```

If the model provides an API to fetch a user explicitly by account (e.g. `findUserByName(m_account)` or similar), consider replacing `m_model->currentUser()` with that call when initializing `user` to make the association with the authenticating account explicit.
</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 thread src/lightdm-deepin-greeter/greeterworker.cpp Outdated
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码移除了不安全的本地shadow文件读取逻辑并修复了空指针风险,整体质量优秀
逻辑正确且无安全漏洞,得满分

■ 【详细分析】

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

在 onAuthFinished 函数中增加了对 m_model->currentUser() 的空指针检查,避免了原代码在用户对象不存在时直接调用方法导致的潜在崩溃;在 checkAccount 函数中补充了新用户加入本地 model 的逻辑,确保后续流程能正确找到用户对象,逻辑严密
建议:保持现有的防御性编程习惯

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

删除了冗余且违反架构设计的 updatePasswordExpiredStateBySPName 函数及其声明,降低了代码复杂度;新增的注释准确解释了域管用户首次登录的处理意图,提升了可读性
建议:继续保持

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

移除了通过 getspnam 同步读取 /etc/shadow 文件的系统调用,该调用在复杂网络环境或大量用户场景下可能因 NSS 模块加载导致明显阻塞,改为依赖后端模型数据提升了响应速度
建议:继续保持

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次提交彻底移除了 greeter 进程直接调用 getspnam 读取 /etc/shadow 敏感密码文件的行为,消除了越权读取敏感信息的攻击面,同时补充了空指针保护,未引入任何新的安全风险

  • 建议:继续坚持由后端服务统一提供密码过期状态的安全架构设计

■ 【改进建议代码示例】

// 当前代码已经非常完善,此处仅展示保留的防御性检查最佳实践
void GreeterWorker::onAuthFinished()
{
    qCInfo(DDE_SHELL) << "Auth finished";
    if (m_greeter->inAuthentication()) {
        m_greeter->respond(m_authFramework->AuthSessionPath(m_account) + QString(";") + m_password);
        // 确保用户对象存在后再进行状态更新和判断
        if (m_model->currentUser()) {
            m_model->currentUser()->updatePasswordExpiredInfo();
            if (m_model->currentUser()->expiredState() == User::ExpiredAlready) {
                changePasswd();
            }
        }
    } else {
        // 其他逻辑
    }
}

deepin-ci-robot added a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Jun 29, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

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

// 新发现的用户(如域管用户首次登录),加入本地 model,
// 确保后续 createAuthentication 中 findUserByName 能找到
if (!originalUsePtr) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

合并后,在snipe仓库提一笔大于dde-daemon修改的版本依赖。

deepin-ci-robot added a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Jun 29, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

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

1. Remove `updatePasswordExpiredStateBySPName()` function that used
glibc `getspnam` to check password expiry, as it was only needed for
first-time domain user login
2. Ensure new domain users discovered in `checkAccount()` are
added to the local model immediately, so subsequent operations like
`createAuthentication()` can find them via `findUserByName()`
3. Fall back to using `updatePasswordExpiredInfo()` through existing
model interfaces for all users, including newly discovered ones
4. Fix a crash in `onAuthFinished()` where `m_model->currentUser()`
could be null when checking `expiredState()`

Log: Fix domain user first login; remove redundant glibc password expiry
logic

Influence:
1. Test domain user first login scenario to verify password expiry info
is correctly synced
2. Test normal user login to ensure no regression with password expiry
checks
3. Verify no crashes occur during authentication finish for any user
type
4. Test multi-user switching, including between local and domain users

fix: 移除基于 glibc 的密码过期检查并修正域管用户处理

1. 移除 `updatePasswordExpiredStateBySPName()` 函数,该函数使用 glibc 的
`getspnam` 检查密码过期,只在域管用户首次登录时需要
2. 确保在 `checkAccount()` 中新发现的域管用户立即加入本地 model,使得后
续 `createAuthentication()` 等操作能通过 `findUserByName()` 找到
3. 所有用户(包括新发现用户)都回退到通过现有 model 接口使用
`updatePasswordExpiredInfo()`
4. 修复 `onAuthFinished()` 中 `m_model->currentUser()` 可能为空时检查
`expiredState()` 导致的崩溃

Log: 修复域管用户首次登录;移除冗余的 glibc 密码过期逻辑

Influence:
1. 测试域管用户首次登录场景,验证密码过期信息是否正确同步
2. 测试普通用户登录,确保密码过期检查无回归
3. 验证任意用户在认证结束时不会发生崩溃
4. 测试多用户切换,包括本地用户和域管用户之间的切换

PMS: BUG-367143
Change-Id: Icbbe23ae6246d4e60c1dbc34d35a4f414d86e8df
deepin-ci-robot added a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Jun 29, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

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

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xionglinlin, yixinshark

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

@xionglinlin xionglinlin merged commit 25c7e36 into linuxdeepin:master Jul 6, 2026
16 of 17 checks passed
xionglinlin pushed a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Jul 6, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#69
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.

3 participants