Skip to content

fix: permission for save audit plan#3078

Merged
LordofAvernus merged 2 commits intomainfrom
fix_save_audit_plan
Jul 4, 2025
Merged

fix: permission for save audit plan#3078
LordofAvernus merged 2 commits intomainfrom
fix_save_audit_plan

Conversation

@Jarvis1105
Copy link
Copy Markdown
Contributor

@Jarvis1105 Jarvis1105 commented Jul 3, 2025

User description

assign in @LordofAvernus

关联的 issue

https://github.com/actiontech/sqle-ee/issues/2387#issuecomment-3032010540

描述你的变更

如果之前其他人在数据源上开启了扫描任务,该用户看不到,尝试开启时报错

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 修改权限检查接口调用参数,删除多余opType

  • 重构权限检查函数,实现多权限遍历校验

  • 更新v1和v2接口中的权限校验调用逻辑

  • 优化代码防止空指针异常


Changes diagram

flowchart LR
  A["Controller API调用"]
  B["GetInstanceAuditPlanIfCurrentUserCanView"]
  C["遍历多权限校验"]
  D["返回审核计划详情"]
  A -- "调用权限检查" --> B
  B -- "检查多种权限" --> C
  C -- "验证并返回结果" --> D
Loading

Changes walkthrough 📝

Relevant files
Bug fix
instance_audit_plan.go
调整实例审核计划权限校验接口调用                                                                                 

sqle/api/controller/v1/instance_audit_plan.go

  • 删除函数调用中的opType参数
  • 统一更新各API权限校验调用
+6/-6     
Enhancement
project_permission.go
重构实例审核计划权限检查逻辑                                                                                     

sqle/api/controller/v1/project_permission.go

  • 修改GetInstanceAuditPlanIfCurrentUserCanView签名
  • opTypes数组替代单一权限参数
  • +4/-4     
    instance_audit_plan.go
    更新实例审核计划接口权限逻辑                                                                                     

    sqle/api/controller/v2/instance_audit_plan.go

    • 更新调用权限检查函数以适应新签名
    • 增加SQL管控权限类型检测
    +3/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    github-actions Bot commented Jul 3, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 8b6f720
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    修复未初始化用户变量

    建议检查调用 GetCanOperationInstances 时传递的用户参数。当前代码直接传入未初始化的 user 变量,可能导致 nil
    指针异常或运行时崩溃。请确保从 c 的上下文中正确获取当前用户,例如通过 c.Get("user") 获取,并传递给该函数。

    sqle/api/controller/v1/project_permission.go [332-337]

     opTypes := []dmsV1.OpPermissionType{dmsV1.OpPermissionTypeViewOtherAuditPlan, dmsV1.OpPermissionTypeSaveAuditPlan}
    +user := c.Get("user") // 从上下文中获取当前用户
     for _, opType := range opTypes {
         dbServiceReq := &dmsV2.ListDBServiceReq{
             ProjectUid: projectId,
         }
         instances, err := GetCanOperationInstances(c.Request().Context(), user, dbServiceReq, opType)
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion accurately identifies a critical issue in which the undeclared user variable is passed to GetCanOperationInstances, potentially causing a nil pointer dereference. The improved code properly retrieves user from the context via c.Get("user"), making the fix both relevant and effective.

    High

    Previous suggestions

    Suggestions up to commit d5b66d6
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    去重合并实例ID

    建议在将通过 SaveAuditPlan
    权限获取的实例ID与其他实例ID合并后,对合并后的数组进行去重处理,以防止重复数据可能引发潜在错误或性能问题。这样可以确保后续操作只针对唯一的实例ID进行处理。

    sqle/api/controller/v2/instance_audit_plan.go [253-254]

     saveAuditPlanInstanceIds := up.GetInstancesByOP(dmsCommonV1.OpPermissionTypeSaveAuditPlan)
    -accessibleInstanceId = append(accessibleInstanceId, saveAuditPlanInstanceIds...)
    +combinedInstances := append(accessibleInstanceId, saveAuditPlanInstanceIds...)
    +instanceSet := make(map[string]struct{})
    +for _, id := range combinedInstances {
    +    instanceSet[id] = struct{}{}
    +}
    +uniqueInstances := make([]string, 0, len(instanceSet))
    +for id := range instanceSet {
    +    uniqueInstances = append(uniqueInstances, id)
    +}
    +accessibleInstanceId = uniqueInstances
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion adds deduplication after merging instance IDs, addressing potential duplicate entries that could affect subsequent operations, thereby enhancing overall data integrity.

    Medium
    更新错误返回信息

    建议根据实际权限检查结果返回更准确的错误提示,而不是固定使用 dmsV1.OpPermissionTypeViewOtherAuditPlan。这样可以避免当
    SaveAuditPlan 权限检查失败时误导用户。请修改返回错误时使用动态确定的权限类型描述。

    sqle/api/controller/v1/project_permission.go [347]

    -return ap, false, errors.NewUserNotPermissionError(dmsV1.GetOperationTypeDesc(dmsV1.OpPermissionTypeViewOtherAuditPlan))
    +// 如果所有权限检查失败,则返回综合的错误提示
    +return ap, false, errors.NewUserNotPermissionError("缺少必要的审核计划访问权限")
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion replaces the dynamic error message with a hard-coded one, which may improve message clarity slightly but reduces flexibility in reflecting actual permission issues.

    Low

    Comment thread sqle/api/controller/v2/instance_audit_plan.go Outdated
    @LordofAvernus LordofAvernus merged commit 9dd221c into main Jul 4, 2025
    4 checks passed
    @Jarvis1105 Jarvis1105 deleted the fix_save_audit_plan branch July 4, 2025 03:30
    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