Skip to content

Conversation

@caixr23
Copy link
Contributor

@caixr23 caixr23 commented Nov 5, 2025

Security options are given default values based on configuration items

pms: BUG-327883

Summary by Sourcery

Provide defaults for enterprise network security options by adding API methods to retrieve configured EAP authentication settings and integrating them into the QML security section.

New Features:

  • Expose default EAP authentication mode and inner auth method via new Q_INVOKABLE NetManager methods

Bug Fixes:

  • Apply configured default values for eapType and phase2-auth in SectionSecret.qml to ensure security options initialize correctly

Enhancements:

  • Delegate wpaEapAuthen and wpaEapAuthmethod calls through NetManagerPrivate and NetManagerThreadPrivate to ConfigSetting
  • Automatically update the phase2-auth selection index when eapType changes in the UI

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码进行审查,主要从代码逻辑、性能、安全性等方面分析:

  1. 代码逻辑审查:
  • 在 SectionSecret.qml 中,新增的 updateIndex() 函数处理了认证方式的索引更新逻辑,考虑了默认值和边界情况(cindex < 0),逻辑严谨。
  • 使用 Connections 来监听 eapType 的变化并触发更新,符合 QML 的响应式编程模式。
  1. 性能分析:
  • 新增的函数调用链较长:QML -> NetManager -> NetManagerPrivate -> NetManagerThreadPrivate -> ConfigSetting,可能带来一定的性能开销。
  • wpaEapAuthmethod() 在 NetManagerThreadPrivate 中转换为小写,这个转换操作应该考虑缓存结果,避免重复调用时重复转换。
  1. 安全性考虑:
  • 在 QML 中直接访问 dccData.manager.wpaEapAuthen(),需要确保这些方法返回值是经过验证的。
  • 建议在 NetManager 中对返回值进行输入验证和清理,防止注入攻击。

改进建议:

  1. 性能优化:
// 在 NetManagerThreadPrivate 中缓存转换结果
QString NetManagerThreadPrivate::wpaEapAuthmethod() const
{
    static QString cachedMethod = ConfigSetting::instance()->wpaEapAuthmethod().toLower();
    return cachedMethod;
}
  1. 安全性增强:
// 在 NetManager 中添加输入验证
QString NetManager::wpaEapAuthen() const
{
    Q_D(const NetManager);
    QString auth = d->wpaEapAuthen();
    // 添加验证逻辑
    if (!isValidAuthMethod(auth)) {
        return getDefaultAuthMethod();
    }
    return auth;
}
  1. 代码质量改进:
  • 建议为新增的方法添加更详细的文档注释,说明返回值的具体格式和可能的值。
  • 可以考虑在 QML 中添加错误处理机制,处理可能的异常情况。
  1. 架构优化:
  • 考虑引入配置缓存机制,避免频繁访问底层配置。
  • 可以考虑使用单例模式管理配置信息,减少重复的实例化开销。
  1. 错误处理:
function updateIndex() {
    try {
        var cindex = (root.config802_1x && root.config802_1x.hasOwnProperty("phase2-auth")) 
            ? indexOfValue(root.config802_1x["phase2-auth"]) 
            : indexOfValue(dccData.manager.wpaEapAuthmethod().toLowerCase());
        currentIndex = cindex < 0 ? 0 : cindex
    } catch (e) {
        console.error("Failed to update index:", e);
        currentIndex = 0;
    }
}

这些改进建议可以提升代码的健壮性、性能和可维护性。

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 5, 2025

Reviewer's Guide

This PR introduces two new QML-accessible methods to fetch default WPA-EAP authentication settings from the configuration and updates the UI component to use these dynamic defaults instead of hardcoded values.

Sequence diagram for fetching WPA-EAP authentication defaults

sequenceDiagram
    participant QML_UI
    participant NetManager
    participant NetManagerPrivate
    participant NetManagerThreadPrivate
    participant ConfigSetting
    QML_UI->>NetManager: wpaEapAuthen()
    NetManager->>NetManagerPrivate: wpaEapAuthen()
    NetManagerPrivate->>NetManagerThreadPrivate: wpaEapAuthen()
    NetManagerThreadPrivate->>ConfigSetting: wpaEapAuthen()
    ConfigSetting-->>NetManagerThreadPrivate: default value
    NetManagerThreadPrivate-->>NetManagerPrivate: default value
    NetManagerPrivate-->>NetManager: default value
    NetManager-->>QML_UI: default value
    QML_UI->>NetManager: wpaEapAuthmethod()
    NetManager->>NetManagerPrivate: wpaEapAuthmethod()
    NetManagerPrivate->>NetManagerThreadPrivate: wpaEapAuthmethod()
    NetManagerThreadPrivate->>ConfigSetting: wpaEapAuthmethod()
    ConfigSetting-->>NetManagerThreadPrivate: default value
    NetManagerThreadPrivate-->>NetManagerPrivate: default value
    NetManagerPrivate-->>NetManager: default value
    NetManager-->>QML_UI: default value
Loading

Class diagram for new WPA-EAP authentication methods

classDiagram
    class NetManager {
        +QString wpaEapAuthen() const
        +QString wpaEapAuthmethod() const
    }
    class NetManagerPrivate {
        +QString wpaEapAuthen() const
        +QString wpaEapAuthmethod() const
    }
    class NetManagerThreadPrivate {
        +QString wpaEapAuthen() const
        +QString wpaEapAuthmethod() const
    }
    NetManager --> NetManagerPrivate
    NetManagerPrivate --> NetManagerThreadPrivate
    NetManagerThreadPrivate --> ConfigSetting
    class ConfigSetting {
        +QString wpaEapAuthen() const
        +QString wpaEapAuthmethod() const
    }
Loading

File-Level Changes

Change Details Files
Expose default WPA-EAP auth settings through the NetManager API
  • Add Q_INVOKABLE getters wpaEapAuthen and wpaEapAuthmethod in NetManager header
  • Implement those getters in NetManager.cpp forwarding to the private d-pointer
  • Declare and implement matching methods in NetManagerPrivate to forward to the thread layer
  • Add getters in NetManagerThreadPrivate to return values from ConfigSetting
net-view/operation/netmanager.h
net-view/operation/netmanager.cpp
net-view/operation/private/netmanager_p.h
net-view/operation/private/netmanagerthreadprivate.h
net-view/operation/private/netmanagerthreadprivate.cpp
Make SectionSecret.qml use configuration-driven defaults for EAP settings
  • Replace hardcoded “tls” type with dccData.manager.wpaEapAuthen() for initial eapType
  • Introduce updateIndex() to compute phase2-auth index with fallback to wpaEapAuthmethod()
  • Add Connections on eapTypeChanged to recompute the phase2-auth selection dynamically
dcc-network/qml/SectionSecret.qml

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 and they look great!


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.

@caixr23 caixr23 requested a review from mhduiy November 5, 2025 08:18
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, mhduiy

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

… items

Security options are given default values based on configuration items

pms: BUG-327883
@caixr23 caixr23 merged commit 553b665 into linuxdeepin:master Nov 6, 2025
13 of 14 checks passed
@caixr23 caixr23 deleted the BUG-327883 branch November 6, 2025 08:23
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