Skip to content

fix: Fix the mysql root account permission switchover exception #8378

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.

Sign up for GitHub

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

Merged
f2c-ci-robot merged 1 commit into from
Apr 11, 2025
Merged

fix: Fix the mysql root account permission switchover exception #8378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions backend/utils/mysql/client/local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,19 +188,18 @@ func (r *Local) ChangeAccess(info AccessChangeInfo) error {
info.Name = "*"
info.Password = r.Password
}
if info.Permission == info.OldPermission {
return nil
}
if err := r.Delete(DeleteInfo{
Version: info.Version,
Username: info.Username,
Permission: info.OldPermission,
ForceDelete: true,
Timeout: 300}); err != nil {
return err
}
if info.Username == "root" {
return nil
if info.Permission != info.OldPermission {
if err := r.Delete(DeleteInfo{
Version: info.Version,
Username: info.Username,
Permission: info.OldPermission,
ForceDelete: true,
Timeout: 300}); err != nil {
return err
}
if info.Username == "root" {
return nil
}
}
if err := r.CreateUser(CreateInfo{
Name: info.Name,
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 11, 2025

Choose a reason for hiding this comment

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

The given code snippet contains two main logic branches:

  1. Regular Branch:

    • This branch checks if info.Permission is equal to info.OldPermission.
    • If they are equal, it returns nil, effectively skipping the deletion step.

  2. Special Case for Root User:

    • If the current user (info.Username) is "root", it also skips the deletion process.

Potential Issues and Improvements:

  1. Code Reorganization:

    • It might be beneficial to extract the common logic of checking if info.Permission == info.OldPermission into a separate function call to improve readability and maintainability.

  2. Error Handling:

    • The function does not handle errors returned by CreateUser. Although it catches errors from DeleteInfo, handling other functions should also be considered within this scope.

  3. Documentation/Comments:

    • Adding comments explaining each section and key decisions could help clarify the purpose of different parts of the code, especially for others who may read this later.

  4. Variable Naming:

    • Consistent naming conventions could make the code more readable, such as using camelCase for variable names rather than underscores.

Overall, the overall structure looks clean and well-organized, but these minor improvements can enhance its clarity and robustness.

Expand Down
25 changes: 12 additions & 13 deletions backend/utils/mysql/client/remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,18 @@ func (r *Remote) ChangeAccess(info AccessChangeInfo) error {
info.Name = "*"
info.Password = r.Password
}
if info.Permission == info.OldPermission {
return nil
}
if err := r.Delete(DeleteInfo{
Version: info.Version,
Username: info.Username,
Permission: info.OldPermission,
ForceDelete: true,
Timeout: 300}); err != nil {
return err
}
if info.Username == "root" {
return nil
if info.Permission != info.OldPermission {
if err := r.Delete(DeleteInfo{
Version: info.Version,
Username: info.Username,
Permission: info.OldPermission,
ForceDelete: true,
Timeout: 300}); err != nil {
return err
}
if info.Username == "root" {
return nil
}
}
if err := r.CreateUser(CreateInfo{
Name: info.Name,
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 11, 2025

Choose a reason for hiding this comment

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

The provided code has several optimizations and corrections:

  1. The if statement that checks if info.Permission == info.OldPermission is unnecessary if it returns before calling r.Delete. You can move the call to r.CreateUser outside this block.

  2. There should be no need to create an empty user object (CreateInfo{}) when creating the root user since the Name field is already set. This redundancy can be removed.

  3. Ensure that there are proper error handling around critical operations like r.CreateUser() in case of any failure during creation.

This refactored version of the function ensures cleaner logic by reducing nested structures where possible:

func (r *Remote) ChangeAccess(info AccessChangeInfo) error {
	if info.Username == "root" && info.Permission != info.OldPermission {
		return nil
	}

	if err := r.Delete(DeleteInfo{
		Version:     info.Version,
		Username:    info.Username,
		Permission:  info.OldPermission,
		ForceDelete: true,
		Timeout:     300}); err != nil {
		return err
	}

	return r.CreateUser(CreateInfo{Name: info.Name})
}

Expand Down
8 changes: 8 additions & 0 deletions frontend/src/views/database/mysql/password/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ const changeVisible = ref(false);
type FormInstance = InstanceType<typeof ElForm>;
const changeFormRef = ref<FormInstance>();
const title = ref();
const oldPrivilege = ref();
const oldPrivilegeIPs = ref();
const changeForm = reactive({
id: 0,
from: '',
Expand Down Expand Up @@ -124,6 +126,8 @@ const acceptParams = (params: DialogProps): void => {
changeForm.privilegeIPs = params.privilegeIPs;
changeForm.value = params.value;
changeVisible.value = true;
oldPrivilege.value = params.privilege;
oldPrivilegeIPs.value = params.privilegeIPs;
};
const emit = defineEmits<{ (e: 'search'): void }>();

Expand Down Expand Up @@ -167,6 +171,10 @@ const submitChangeInfo = async (formEl: FormInstance | undefined) => {
}
return;
}
if (changeForm.privilege === oldPrivilege.value && changeForm.privilegeIPs === oldPrivilegeIPs.value) {
changeVisible.value = false;
return;
}
if (changeForm.privilege !== 'ip') {
param.value = changeForm.privilege;
} else {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 11, 2025

Choose a reason for hiding this comment

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

There are no major irregularities or obvious issues in this code snippet, but there are some areas that can be optimized or improved:

  1. Consolidate Code: The two else blocks under the conditional checks can be combined to reduce redundancy.

  2. Empty Check: It's good practice to add an empty string check before comparing strings to avoid unexpected behavior when one of them might be null or undefined.

  3. Simplify Boolean Logic: You could simplify the boolean logic by checking for equality between arrays explicitly instead of comparing their reference values.

  4. Use Object Destructuring: If these references will always contain properties with the same names, you could destructure them directly to make the code slightly more concise.

Here is a revised version incorporating these suggestions:

if (formEl && formEl.validate(async valid => {
  if (!valid) {            
      return;
  }      
  await validateEmail(changeForm.from);

  if (changeForm.privilege === oldPrivilege.value && equalArrays(changeForm.privilegeIPs, oldPrivilegeIPs.value)) {
      changeVisible.value = false;
      return;
  }

  if (changeForm.privilege !== 'ip') {
      param.value = changeForm.privilege;
  } else {
      // continue processing
  }
})):

Function equalArrays:
If you don't already have such a function, here it is as a helper to compare array elements:

function equalArrays(a, b) {
  if (a.length !== b.length)
    return false;

  for (let i = 0; i < a.length; i++) 
      if (a[i] !== b[i])    
          return false;

  return true;
}

These improvements should enhance readability, maintainability, and potentially performance marginally.

Expand Down
Loading