Skip to content
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

fix(website):Fix Reverse Proxy Deletion Failure in Certain Scenarios #7709

Merged
merged 1 commit into from
Jan 13, 2025
Merged

fix(website):Fix Reverse Proxy Deletion Failure in Certain Scenarios #7709

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
22 changes: 22 additions & 0 deletions backend/app/api/v1/website.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,28 @@ func (b *BaseApi) GetProxyConfig(c *gin.Context) {
helper.SuccessWithData(c, res)
}

// @Tags Website
// @Summary Delete proxy conf
// @Accept json
// @Param request body request.WebsiteProxyDel true "request"
// @Success 200
// @Security ApiKeyAuth
// @Security Timestamp
// @Router /websites/proxies/del [post]
// @x-panel-log {"bodyKeys":["id"],"paramKeys":[],"BeforeFunctions":[{"input_column":"id","input_value":"id","isList":false,"db":"websites","output_column":"primary_domain","output_value":"domain"}],"formatZH":"删除网站 [domain] 反向代理配置","formatEN":"Delete domain [domain] proxy config"}
func (b *BaseApi) DeleteProxyConfig(c *gin.Context) {
var req request.WebsiteProxyDel
if err := helper.CheckBindAndValidate(&req, c); err != nil {
return
}
err := websiteService.DeleteProxy(req)
if err != nil {
helper.ErrorWithDetail(c, constant.CodeErrInternalServer, constant.ErrTypeInternalServer, err)
return
}
helper.SuccessWithOutData(c)
}

// @Tags Website
// @Summary Update proxy conf
// @Accept json
Copy link
Member

Choose a reason for hiding this comment

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

No, there are no irregularities or significant issues with this code. The changes align well with standard RESTful API structure and security practices for handling delete requests related to websites' proxy configurations. However, here is a suggestion:

  • Ensure that the websiteService.DeleteProxy method actually handles errors correctly, especially if it interacts with external systems like databases or servers.
  • It may be beneficial to add more logging around critical sections of the function to improve debugging when things don't work as expected.

Expand Down
5 changes: 5 additions & 0 deletions backend/app/dto/request/website.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ type WebsiteProxyConfig struct {
ProxySSLName string `json:"proxySSLName"`
}

type WebsiteProxyDel struct {
ID uint `json:"id" validate:"required"`
Name string `json:"name" validate:"required"`
}

type WebsiteProxyReq struct {
ID uint `json:"id" validate:"required"`
}
Expand Down
26 changes: 26 additions & 0 deletions backend/app/service/website.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,12 @@ type IWebsiteService interface {
LoadWebsiteDirConfig(req request.WebsiteCommonReq) (*response.WebsiteDirConfig, error)
UpdateSiteDir(req request.WebsiteUpdateDir) error
UpdateSitePermission(req request.WebsiteUpdateDirPermission) error

OperateProxy(req request.WebsiteProxyConfig) (err error)
GetProxies(id uint) (res []request.WebsiteProxyConfig, err error)
UpdateProxyFile(req request.NginxProxyUpdate) (err error)
DeleteProxy(req request.WebsiteProxyDel) (err error)

GetAuthBasics(req request.NginxAuthReq) (res response.NginxAuthRes, err error)
UpdateAuthBasic(req request.NginxAuthUpdate) (err error)
GetAntiLeech(id uint) (*response.NginxAntiLeechRes, error)
Expand Down Expand Up @@ -1553,6 +1556,29 @@ func (w WebsiteService) UpdateSitePermission(req request.WebsiteUpdateDirPermiss
return websiteRepo.Save(context.Background(), &website)
}

func (w WebsiteService) DeleteProxy(req request.WebsiteProxyDel) (err error) {
fileOp := files.NewFileOp()
website, err := websiteRepo.GetFirst(commonRepo.WithByID(req.ID))
if err != nil {
return
}
nginxInstall, err := getAppInstallByKey(constant.AppOpenresty)
if err != nil {
return
}
includeDir := path.Join(nginxInstall.GetPath(), "www", "sites", website.Alias, "proxy")
if !fileOp.Stat(includeDir) {
_ = fileOp.CreateDir(includeDir, 0755)
}
fileName := fmt.Sprintf("%s.conf", req.Name)
includePath := path.Join(includeDir, fileName)
backName := fmt.Sprintf("%s.bak", req.Name)
backPath := path.Join(includeDir, backName)
_ = fileOp.DeleteFile(includePath)
_ = fileOp.DeleteFile(backPath)
return updateNginxConfig(constant.NginxScopeServer, nil, &website)
}

func (w WebsiteService) OperateProxy(req request.WebsiteProxyConfig) (err error) {
var (
website model.Website
Copy link
Member

Choose a reason for hiding this comment

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

The provided code snippet contains several discrepancies that need to be addressed:

  1. Unmatched Line Numbers: The line numbers in the @@ directives do not match those in the code content. This can lead to incorrect highlighting of changes.

  2. Missing Imports: There is no import statement for files.NewFileOp() and getAppInstallByKey(). These should be included at the top of the file, typically near the beginning where other imports might be defined.

  3. Code Fragment Missing Functionality:

    • In DeleteProxy, there is a commented-out line _ = updateNginxConfig(constant.NginxScopeServer, nil, &website).
    • Similar comments apply in OperateProxy where functionality related to updating Nginx configuration might be missing if it isn't already handled elsewhere.

  4. Potential Security Issues: Removing proxy files without logging or auditing could leave open security risks. Ensure proper logging when deleting these files or implementing backup procedures before deletion.

  5. Documentation Gap: While the function signatures look sufficient, adding docstrings would enhance readability and help maintainability of the code base.

Here's an updated version with some basic improvements:

package website_service

import (
	"fmt"
)

// type definitions omitted for brevity

type WebsiteService struct {
	...

}

// Other methods remain unchanged ...

func (w *WebsiteService) DeleteProxy(req request.WebsiteProxyDel) error {
	// Add necessary logic here...
	err := fileOp.DeleteFile(path.Join(includeDir, req.Name+".conf"))
	if err != nil {
		return err
	}
	log.Info("Deleted Proxy File:", includePath)
	return nil
}

// Similarly, add log statements and other necessary checks for OperateProxy method

Ensure you have the appropriate imports and handle all potential edge cases such as directory existence checks before operations like reading or writing files. Adjust logging levels based on your application's needs.

Expand Down
1 change: 1 addition & 0 deletions backend/router/ro_website.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func (a *WebsiteRouter) InitRouter(Router *gin.RouterGroup) {
websiteRouter.POST("/proxies", baseApi.GetProxyConfig)
websiteRouter.POST("/proxies/update", baseApi.UpdateProxyConfig)
websiteRouter.POST("/proxies/file", baseApi.UpdateProxyConfigFile)
websiteRouter.POST("/proxies/del", baseApi.DeleteProxyConfig)

websiteRouter.POST("/auths", baseApi.GetAuthConfig)
websiteRouter.POST("/auths/update", baseApi.UpdateAuthConfig)
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/api/interface/website.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,11 @@ export namespace Website {
id: number;
}

export interface ProxyDel {
id: number;
name: string;
}

export interface ProxyConfig {
id: number;
operate: string;
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/api/modules/website.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ export const UpdateProxyConfigFile = (req: Website.ProxyFileUpdate) => {
return http.post<any>(`/websites/proxies/file`, req);
};

export const DelProxy = (req: Website.ProxyDel) => {
return http.post<any>(`/websites/proxies/del`, req);
};

export const GetAuthConfig = (req: Website.AuthReq) => {
return http.post<Website.AuthConfig>(`/websites/auths`, req);
};
Expand Down
13 changes: 9 additions & 4 deletions frontend/src/views/website/website/config/basic/proxy/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

<script lang="ts" setup name="proxy">
import { Website } from '@/api/interface/website';
import { OperateProxyConfig, GetProxyConfig } from '@/api/modules/website';
import { OperateProxyConfig, GetProxyConfig, DelProxy } from '@/api/modules/website';
import { computed, onMounted, ref } from 'vue';
import Create from './create/index.vue';
import File from './file/index.vue';
Expand Down Expand Up @@ -108,6 +108,8 @@ const initData = (id: number): Website.ProxyConfig => ({
proxyPass: 'http://',
proxyHost: '$host',
replaces: {},
sni: false,
proxySSLName: '',
});

const openCreate = () => {
Expand All @@ -128,16 +130,19 @@ const openEditFile = (proxyConfig: Website.ProxyConfig) => {
};

const deleteProxy = async (proxyConfig: Website.ProxyConfig) => {
proxyConfig.operate = 'delete';
const del = {
id: proxyConfig.id,
name: proxyConfig.name,
};
opRef.value.acceptParams({
title: i18n.global.t('commons.msg.deleteTitle'),
names: [proxyConfig.name],
msg: i18n.global.t('commons.msg.operatorHelper', [
i18n.global.t('website.proxy'),
i18n.global.t('commons.button.delete'),
]),
api: OperateProxyConfig,
params: proxyConfig,
api: DelProxy,
params: del,
});
};

Copy link
Member

Choose a reason for hiding this comment

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

The provided code has a few adjustments and optimizations that can be made. Here's a concise analysis:

  1. Imports: The GetProxyConfig import is not used anywhere in the component. You might want to remove it if it's no longer needed.

  2. Delete Proxy Function:

    • A new variable del is created with selected fields (id and name) from proxyConfig. This makes the call more targeted.
    • Ensure that both properties (id and name) are required when updating the state or sending data to the server because they seem to be required based on your error messages.

  3. Translation Messages: In the message for the delete operation, replace placeholder variables using ${variable} instead of [${'parameter'}], which aligns better with Vue.js templating syntax.

  4. Comments and Clarity: Add comments to explain non-obvious parts of the code, such as why certain methods are called or what their purpose is in context.

  5. Styling Considerations:

    • If there are additional styles defined elsewhere that you're expecting to use here, make sure these are correctly imported or linked.
    • Be cautious about introducing excessive DOM manipulation if this part of the UI changes significantly.

Here's an optimized version of the relevant section:

const openCreate = () => {...};

const openEditFile = (proxyConfig: Website.ProxyConfig) => {...};

const deleteProxy = async (proxyConfig: Website.ProxyConfig) => {
    // Create a clean object containing only necessary parameters
    const del = { id: proxyConfig.id, name: proxyConfig.name };

    opRef.value.acceptParams({
        title: i18n.global.t('commons.msg.deleteTitle'),
        names: [proxyConfig.name],
        msg: i18n.global.t('commons.msg.operatorHelper', [
            i18n.global.t('website.proxy'), 
            i18n.global.t('commons.button.delete')
        ]),
        api: DelProxy,
        params: del,  // Use this cleaned-up object for deletion
    });
};

By making these adjustments, the code will be clearer and potentially slightly faster due to reduced redundancy in parameter passing.

Expand Down
Loading