From e298996d77d95498f29bed0f07477b440acd20ae Mon Sep 17 00:00:00 2001 From: Sora39831 <540587985@qq.com> Date: Mon, 6 Apr 2026 22:12:38 +0800 Subject: [PATCH] Harden admin access for panel APIs --- web/controller/access_control_test.go | 155 ++++++++++++++++++++++++++ web/controller/api.go | 8 +- web/controller/inbound.go | 39 ++++--- web/controller/setting.go | 2 + web/controller/xui.go | 6 +- web/service/inbound.go | 79 +++++++++++++ web/service/inbound_access_test.go | 63 +++++++++++ 7 files changed, 334 insertions(+), 18 deletions(-) create mode 100644 web/controller/access_control_test.go create mode 100644 web/service/inbound_access_test.go diff --git a/web/controller/access_control_test.go b/web/controller/access_control_test.go new file mode 100644 index 00000000..4744f29f --- /dev/null +++ b/web/controller/access_control_test.go @@ -0,0 +1,155 @@ +package controller + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/cookie" + "github.com/gin-gonic/gin" + "github.com/mhsanaei/3x-ui/v2/database" + "github.com/mhsanaei/3x-ui/v2/database/model" + "github.com/mhsanaei/3x-ui/v2/web/global" + "github.com/mhsanaei/3x-ui/v2/web/session" + "github.com/mhsanaei/3x-ui/v2/xray" + "github.com/robfig/cron/v3" +) + +type testWebServer struct { + cron *cron.Cron +} + +func (s *testWebServer) GetCron() *cron.Cron { return s.cron } +func (s *testWebServer) GetCtx() context.Context { return context.Background() } +func (s *testWebServer) GetWSHub() any { return nil } + +func setupControllerTestDB(t *testing.T) { + t.Helper() + tmpDir := t.TempDir() + t.Setenv("XUI_DEBUG", "") + t.Setenv("XUI_DB_FOLDER", tmpDir) + dbPath := filepath.Join(tmpDir, "controller-test.db") + if err := database.InitDBWithPath(dbPath); err != nil { + t.Fatalf("InitDB failed: %v", err) + } + t.Cleanup(func() { + database.CloseDB() + }) +} + +func newTestRouter(t *testing.T) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + + r := gin.New() + store := cookie.NewStore([]byte("test-secret")) + r.Use(sessions.Sessions("3x-ui", store)) + r.Use(func(c *gin.Context) { + c.Set("base_path", "/") + role := c.GetHeader("X-Test-Role") + if role == "" { + return + } + user := &model.User{ + Id: 1, + Username: c.GetHeader("X-Test-Username"), + Role: role, + } + if user.Username == "" { + user.Username = "tester@example.com" + } + session.SetLoginUser(c, user) + }) + return r +} + +func TestXUIController_SettingsPageRequiresAdmin(t *testing.T) { + r := newTestRouter(t) + NewXUIController(r.Group("/")) + + req := httptest.NewRequest(http.MethodGet, "/panel/settings", nil) + req.Header.Set("X-Test-Role", "user") + w := httptest.NewRecorder() + + r.ServeHTTP(w, req) + + if w.Code != http.StatusTemporaryRedirect { + t.Fatalf("expected %d, got %d", http.StatusTemporaryRedirect, w.Code) + } + if got := w.Header().Get("Location"); got != "/panel/user" { + t.Fatalf("expected redirect to /panel/user, got %q", got) + } +} + +func TestAPIController_AdminEndpointsRequireAdmin(t *testing.T) { + global.SetWebServer(&testWebServer{cron: cron.New()}) + + r := newTestRouter(t) + NewAPIController(r.Group("/")) + + for _, path := range []string{ + "/panel/api/inbounds/list", + "/panel/api/server/status", + } { + req := httptest.NewRequest(http.MethodGet, path, nil) + req.Header.Set("X-Test-Role", "user") + req.Header.Set("X-Requested-With", "XMLHttpRequest") + w := httptest.NewRecorder() + + r.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("%s: expected %d, got %d", path, http.StatusForbidden, w.Code) + } + } +} + +func TestAPIController_UserInfoRemainsAvailableToLoggedInUser(t *testing.T) { + setupControllerTestDB(t) + global.SetWebServer(&testWebServer{cron: cron.New()}) + + inboundSettings, err := json.Marshal(map[string]any{ + "clients": []map[string]any{ + {"id": "client-1", "email": "tester@example.com", "enable": true, "subId": "sub-1"}, + }, + }) + if err != nil { + t.Fatalf("marshal inbound settings failed: %v", err) + } + + inbound := &model.Inbound{ + UserId: 1, + Port: 12001, + Protocol: model.VLESS, + Tag: "controller-user-info", + Settings: string(inboundSettings), + } + if err := database.GetDB().Create(inbound).Error; err != nil { + t.Fatalf("create inbound failed: %v", err) + } + if err := database.GetDB().Create(&xray.ClientTraffic{ + InboundId: inbound.Id, + Email: "tester@example.com", + Enable: true, + }).Error; err != nil { + t.Fatalf("create client traffic failed: %v", err) + } + + r := newTestRouter(t) + NewAPIController(r.Group("/")) + + req := httptest.NewRequest(http.MethodGet, "/panel/api/inbounds/userInfo", nil) + req.Header.Set("X-Test-Role", "user") + req.Header.Set("X-Test-Username", "tester@example.com") + w := httptest.NewRecorder() + + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected %d, got %d", http.StatusOK, w.Code) + } +} diff --git a/web/controller/api.go b/web/controller/api.go index 74a6d301..a9ffc369 100644 --- a/web/controller/api.go +++ b/web/controller/api.go @@ -43,10 +43,14 @@ func (a *APIController) initRouter(g *gin.RouterGroup) { // Inbounds API inbounds := api.Group("/inbounds") - a.inboundController = NewInboundController(inbounds) + a.inboundController = &InboundController{} + inbounds.GET("/userInfo", a.inboundController.getUserInfo) + inbounds.Use(a.checkAdmin) + a.inboundController.initRouter(inbounds) // Server API server := api.Group("/server") + server.Use(a.checkAdmin) a.serverController = NewServerController(server) // Users API @@ -55,7 +59,7 @@ func (a *APIController) initRouter(g *gin.RouterGroup) { a.userController = NewUserController(users) // Extra routes - api.GET("/backuptotgbot", a.BackuptoTgbot) + api.GET("/backuptotgbot", a.checkAdmin, a.BackuptoTgbot) } // BackuptoTgbot sends a backup of the panel data to Telegram bot admins. diff --git a/web/controller/inbound.go b/web/controller/inbound.go index 069ca188..fbd915f6 100644 --- a/web/controller/inbound.go +++ b/web/controller/inbound.go @@ -2,6 +2,7 @@ package controller import ( "encoding/json" + "errors" "fmt" "strconv" "time" @@ -12,6 +13,7 @@ import ( "github.com/mhsanaei/3x-ui/v2/web/websocket" "github.com/gin-gonic/gin" + "gorm.io/gorm" ) // InboundController handles HTTP requests related to Xray inbounds management. @@ -48,7 +50,6 @@ func (a *InboundController) initRouter(g *gin.RouterGroup) { g.POST("/resetAllClientTraffics/:id", a.resetAllClientTraffics) g.POST("/delDepletedClients/:id", a.delDepletedClients) g.POST("/import", a.importInbound) - g.GET("/userInfo", a.getUserInfo) g.POST("/onlines", a.onlines) g.POST("/lastOnline", a.lastOnline) g.POST("/updateClientTraffic/:email", a.updateClientTraffic) @@ -73,8 +74,13 @@ func (a *InboundController) getInbound(c *gin.Context) { jsonMsg(c, I18nWeb(c, "get"), err) return } - inbound, err := a.inboundService.GetInbound(id) + user := session.GetLoginUser(c) + inbound, err := a.inboundService.GetInboundForUser(user.Id, user.Role == "admin", id) if err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.obtain"), errors.New("inbound not found")) + return + } jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.obtain"), err) return } @@ -140,7 +146,8 @@ func (a *InboundController) delInbound(c *gin.Context) { jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundDeleteSuccess"), err) return } - needRestart, err := a.inboundService.DelInbound(id) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.DelInboundForUser(user.Id, user.Role == "admin", id) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -150,7 +157,6 @@ func (a *InboundController) delInbound(c *gin.Context) { a.xrayService.SetToNeedRestart() } // Broadcast inbounds update via WebSocket - user := session.GetLoginUser(c) inbounds, _ := a.inboundService.GetInbounds(user.Id) websocket.BroadcastInbounds(inbounds) } @@ -170,7 +176,8 @@ func (a *InboundController) updateInbound(c *gin.Context) { jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err) return } - inbound, needRestart, err := a.inboundService.UpdateInbound(inbound) + user := session.GetLoginUser(c) + inbound, needRestart, err := a.inboundService.UpdateInboundForUser(user.Id, user.Role == "admin", inbound) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -180,7 +187,6 @@ func (a *InboundController) updateInbound(c *gin.Context) { a.xrayService.SetToNeedRestart() } // Broadcast inbounds update via WebSocket - user := session.GetLoginUser(c) inbounds, _ := a.inboundService.GetInbounds(user.Id) websocket.BroadcastInbounds(inbounds) } @@ -250,7 +256,8 @@ func (a *InboundController) addInboundClient(c *gin.Context) { return } - needRestart, err := a.inboundService.AddInboundClient(data) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.AddInboundClientForUser(user.Id, user.Role == "admin", data) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -270,7 +277,8 @@ func (a *InboundController) delInboundClient(c *gin.Context) { } clientId := c.Param("clientId") - needRestart, err := a.inboundService.DelInboundClient(id, clientId) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.DelInboundClientForUser(user.Id, user.Role == "admin", id, clientId) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -292,7 +300,8 @@ func (a *InboundController) updateInboundClient(c *gin.Context) { return } - needRestart, err := a.inboundService.UpdateInboundClient(inbound, clientId) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.UpdateInboundClientForUser(user.Id, user.Role == "admin", inbound, clientId) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -312,7 +321,8 @@ func (a *InboundController) resetClientTraffic(c *gin.Context) { } email := c.Param("email") - needRestart, err := a.inboundService.ResetClientTraffic(id, email) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.ResetClientTrafficForUser(user.Id, user.Role == "admin", id, email) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -343,7 +353,8 @@ func (a *InboundController) resetAllClientTraffics(c *gin.Context) { return } - err = a.inboundService.ResetAllClientTraffics(id) + user := session.GetLoginUser(c) + err = a.inboundService.ResetAllClientTrafficsForUser(user.Id, user.Role == "admin", id) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -390,7 +401,8 @@ func (a *InboundController) delDepletedClients(c *gin.Context) { jsonMsg(c, I18nWeb(c, "pages.inbounds.toasts.inboundUpdateSuccess"), err) return } - err = a.inboundService.DelDepletedClients(id) + user := session.GetLoginUser(c) + err = a.inboundService.DelDepletedClientsForUser(user.Id, user.Role == "admin", id) if err != nil { jsonMsg(c, I18nWeb(c, "somethingWentWrong"), err) return @@ -444,7 +456,8 @@ func (a *InboundController) delInboundClientByEmail(c *gin.Context) { } email := c.Param("email") - needRestart, err := a.inboundService.DelInboundClientByEmail(inboundId, email) + user := session.GetLoginUser(c) + needRestart, err := a.inboundService.DelInboundClientByEmailForUser(user.Id, user.Role == "admin", inboundId, email) if err != nil { jsonMsg(c, "Failed to delete client by email", err) return diff --git a/web/controller/setting.go b/web/controller/setting.go index be0629ba..92bc7b84 100644 --- a/web/controller/setting.go +++ b/web/controller/setting.go @@ -24,6 +24,7 @@ type updateUserForm struct { // SettingController handles settings and user management operations. type SettingController struct { + BaseController settingService service.SettingService userService service.UserService panelService service.PanelService @@ -39,6 +40,7 @@ func NewSettingController(g *gin.RouterGroup) *SettingController { // initRouter sets up the routes for settings management. func (a *SettingController) initRouter(g *gin.RouterGroup) { g = g.Group("/setting") + g.Use(a.checkAdmin) g.POST("/all", a.getAllSetting) g.POST("/defaultSettings", a.getDefaultSettings) diff --git a/web/controller/xui.go b/web/controller/xui.go index 44ff5e1e..607359e5 100644 --- a/web/controller/xui.go +++ b/web/controller/xui.go @@ -30,9 +30,9 @@ func (a *XUIController) initRouter(g *gin.RouterGroup) { g.GET("/", a.index) g.GET("/user", a.user) - g.GET("/inbounds", a.inbounds) - g.GET("/settings", a.settings) - g.GET("/xray", a.xraySettings) + g.GET("/inbounds", a.checkAdmin, a.inbounds) + g.GET("/settings", a.checkAdmin, a.settings) + g.GET("/xray", a.checkAdmin, a.xraySettings) g.GET("/users", a.checkAdmin, a.users) a.settingController = NewSettingController(g) diff --git a/web/service/inbound.go b/web/service/inbound.go index e0daa47f..83362259 100644 --- a/web/service/inbound.go +++ b/web/service/inbound.go @@ -389,6 +389,85 @@ func (s *InboundService) GetInbound(id int) (*model.Inbound, error) { return inbound, nil } +func (s *InboundService) getInboundQueryForUser(userID int, isAdmin bool) *gorm.DB { + db := database.GetDB().Model(model.Inbound{}) + if !isAdmin { + db = db.Where("user_id = ?", userID) + } + return db +} + +func (s *InboundService) GetInboundForUser(userID int, isAdmin bool, id int) (*model.Inbound, error) { + inbound := &model.Inbound{} + if err := s.getInboundQueryForUser(userID, isAdmin).First(inbound, id).Error; err != nil { + return nil, err + } + return inbound, nil +} + +func (s *InboundService) UpdateInboundForUser(userID int, isAdmin bool, inbound *model.Inbound) (*model.Inbound, bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, inbound.Id); err != nil { + return inbound, false, err + } + return s.UpdateInbound(inbound) +} + +func (s *InboundService) DelInboundForUser(userID int, isAdmin bool, id int) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, id); err != nil { + return false, err + } + return s.DelInbound(id) +} + +func (s *InboundService) AddInboundClientForUser(userID int, isAdmin bool, data *model.Inbound) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, data.Id); err != nil { + return false, err + } + return s.AddInboundClient(data) +} + +func (s *InboundService) DelInboundClientForUser(userID int, isAdmin bool, inboundID int, clientID string) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, inboundID); err != nil { + return false, err + } + return s.DelInboundClient(inboundID, clientID) +} + +func (s *InboundService) UpdateInboundClientForUser(userID int, isAdmin bool, data *model.Inbound, clientID string) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, data.Id); err != nil { + return false, err + } + return s.UpdateInboundClient(data, clientID) +} + +func (s *InboundService) ResetClientTrafficForUser(userID int, isAdmin bool, inboundID int, clientEmail string) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, inboundID); err != nil { + return false, err + } + return s.ResetClientTraffic(inboundID, clientEmail) +} + +func (s *InboundService) ResetAllClientTrafficsForUser(userID int, isAdmin bool, id int) error { + if _, err := s.GetInboundForUser(userID, isAdmin, id); err != nil { + return err + } + return s.ResetAllClientTraffics(id) +} + +func (s *InboundService) DelDepletedClientsForUser(userID int, isAdmin bool, id int) error { + if _, err := s.GetInboundForUser(userID, isAdmin, id); err != nil { + return err + } + return s.DelDepletedClients(id) +} + +func (s *InboundService) DelInboundClientByEmailForUser(userID int, isAdmin bool, inboundID int, email string) (bool, error) { + if _, err := s.GetInboundForUser(userID, isAdmin, inboundID); err != nil { + return false, err + } + return s.DelInboundClientByEmail(inboundID, email) +} + // UpdateInbound modifies an existing inbound configuration. // It validates changes, updates the database, and syncs with the running Xray instance. // Returns the updated inbound, whether Xray needs restart, and any error. diff --git a/web/service/inbound_access_test.go b/web/service/inbound_access_test.go new file mode 100644 index 00000000..ba1c8739 --- /dev/null +++ b/web/service/inbound_access_test.go @@ -0,0 +1,63 @@ +package service + +import ( + "errors" + "testing" + + "github.com/mhsanaei/3x-ui/v2/database/model" + "gorm.io/gorm" +) + +func TestGetInboundForUser_DeniesOtherUsers(t *testing.T) { + setupTestDB(t) + + svc := &InboundService{} + inbound := mustCreateInboundWithClients(t, svc, model.Inbound{ + UserId: 2, + Port: 13001, + Protocol: model.VLESS, + Tag: "owned-by-user-2", + }, model.Client{ + ID: "client-1", + Email: "user2@example.com", + Enable: false, + }) + + _, err := svc.GetInboundForUser(1, false, inbound.Id) + if !errors.Is(err, gorm.ErrRecordNotFound) { + t.Fatalf("expected ErrRecordNotFound, got %v", err) + } + + got, err := svc.GetInboundForUser(2, false, inbound.Id) + if err != nil { + t.Fatalf("expected owner to fetch inbound: %v", err) + } + if got.Id != inbound.Id { + t.Fatalf("expected inbound %d, got %d", inbound.Id, got.Id) + } +} + +func TestDelInboundForUser_DeniesOtherUsers(t *testing.T) { + setupTestDB(t) + + svc := &InboundService{} + inbound := mustCreateInboundWithClients(t, svc, model.Inbound{ + UserId: 2, + Port: 13002, + Protocol: model.VLESS, + Tag: "delete-owned-by-user-2", + }, model.Client{ + ID: "client-1", + Email: "user2@example.com", + Enable: false, + }) + + _, err := svc.DelInboundForUser(1, false, inbound.Id) + if !errors.Is(err, gorm.ErrRecordNotFound) { + t.Fatalf("expected ErrRecordNotFound, got %v", err) + } + + if _, err := svc.GetInbound(inbound.Id); err != nil { + t.Fatalf("expected inbound to remain after denied delete: %v", err) + } +}