-
Notifications
You must be signed in to change notification settings - Fork 106
fix: 解决设置快速登录后,切换用户失败的问题 #995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lightdm的state_user文件安全整改后,改为lightdm用户组 但是dde-system-daemon设置快速登录也会修改该文件,通过g_key_file_save_to_file会创建新文件并覆盖旧文件,会将其权限改成root,导致切换用户时修改文件无权限. Log: 解决切换用户失败的问题 PMS: BUG-339935 Influence: quicklogin
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures the LightDM greeter state file created/updated during quick login configuration keeps the correct root:lightdm ownership to avoid permission issues when switching users. Sequence diagram for SetQuickLogin updating LightDM greeter state file ownershipsequenceDiagram
participant Caller
participant DisplayManager as accounts1_display_manager
participant OS as os
participant UserPkg as user
participant Strconv as strconv
Caller->>DisplayManager: SetQuickLogin(username, enabled)
alt state file directory does not exist
DisplayManager->>OS: MkdirAll(stateFileDir)
OS-->>DisplayManager: error or nil
alt error
DisplayManager-->>Caller: error
else success
note over DisplayManager: continue to update greeter state file
end
end
DisplayManager->>DisplayManager: setIniStringList(GreeterStateFile, group, key, usernames)
DisplayManager-->>Caller: error or nil
opt defer keep ownership root:lightdm
DisplayManager->>UserPkg: Lookup(lightdm)
UserPkg-->>DisplayManager: lightdmUser or error
alt lookup success
DisplayManager->>Strconv: Atoi(lightdmUser.Gid)
Strconv-->>DisplayManager: lightdmGID or error
alt conversion success
DisplayManager->>OS: Chown(GreeterStateFile, 0, lightdmGID)
OS-->>DisplayManager: nil or error (ignored)
else conversion error
note over DisplayManager: skip Chown
end
else lookup error
note over DisplayManager: skip Chown
end
end
Flow diagram for SetQuickLogin LightDM greeter state file ownership handlingflowchart TD
A[Start SetQuickLogin] --> B{State file dir exists?}
B -->|No| C[Create directory with MkdirAll]
C --> D{MkdirAll error?}
D -->|Yes| Z[Return error]
D -->|No| E[Proceed to update greeter state file]
B -->|Yes| E
E --> F[Call setIniStringList to write GreeterStateFile]
F --> G[Deferred: Lookup lightdm user]
G --> H{Lookup success?}
H -->|No| I[Skip Chown]
H -->|Yes| J[Convert lightdmUser.Gid with Atoi]
J --> K{Atoi success?}
K -->|No| I
K -->|Yes| L[Call Chown on GreeterStateFile with uid 0 and gid lightdmGID]
L --> M[End defer]
I --> M
M --> N[Return from SetQuickLogin]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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:
- The
deferthat restoresroot:lightdmownership is defined only in the branch that creates the directory/file, so existing files that are overwritten won’t get their group fixed; consider moving thedefer(or a non-deferred call) so it always runs whenGreeterStateFileis modified. - User lookup for
lightdmis done on every call and only for the deferred chown; you could move the lookup (and GID parsing) outside thedeferand reuse the result to avoid repeated work and make error handling clearer. - Currently, failures to look up
lightdmor to change ownership are silently ignored; consider logging these specific failures so misconfiguration of thelightdmuser or file permissions can be diagnosed in the field.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `defer` that restores `root:lightdm` ownership is defined only in the branch that creates the directory/file, so existing files that are overwritten won’t get their group fixed; consider moving the `defer` (or a non-deferred call) so it always runs when `GreeterStateFile` is modified.
- User lookup for `lightdm` is done on every call and only for the deferred chown; you could move the lookup (and GID parsing) outside the `defer` and reuse the result to avoid repeated work and make error handling clearer.
- Currently, failures to look up `lightdm` or to change ownership are silently ignored; consider logging these specific failures so misconfiguration of the `lightdm` user or file permissions can be diagnosed in the field.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, yixinshark 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 |
lightdm的state_user文件安全整改后,改为lightdm用户组
但是dde-system-daemon设置快速登录也会修改该文件,通过g_key_file_save_to_file会创建新文件并覆盖旧文件,会将其权限改成root,导致切换用户时修改文件无权限.
Log: 解决切换用户失败的问题
PMS: BUG-339935
Influence: quicklogin
Summary by Sourcery
Bug Fixes: