From 111ef8ea33c9a0302d544cde10312f306e381e4d Mon Sep 17 00:00:00 2001 From: "GL.iNet-Yongping.Xie" Date: Wed, 14 Oct 2026 00:38:46 -0700 Subject: [PATCH] fix(api): check device access on connect/cmd/web These routes only verified that a session existed, so any logged-in user could drive any device by id, including ungrouped ones the device list never showed them. Apply the same visibility rule as the list: admins reach everything, a regular user only devices in a group linked to one of their user groups. Denials answer with the error page, a bare 403 for websocket upgrades, and err 1004 for /cmd. Also drop the two shortcuts in httpAuth that skipped authentication when no password was configured or when local-auth was off and the request came from loopback -- behind a reverse proxy every request looks local. Both settings now log a warning instead. Signed-off-by: GL.iNet-Yongping.Xie --- internal/server/api.go | 151 ++++++++++++++++++++++++++++--------- internal/server/command.go | 14 ++-- 2 files changed, 123 insertions(+), 42 deletions(-) diff --git a/internal/server/api.go b/internal/server/api.go index 2a88475..4727c8b 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -34,6 +34,7 @@ import ( "path" "rttys/internal/domain/device" "rttys/internal/domain/devicelog" + "rttys/internal/domain/identity" "rttys/internal/domain/notification" "rttys/internal/domain/permission" "rttys/internal/domain/user" @@ -200,11 +201,16 @@ func (srv *RttyServer) ListenAPI() error { r.Use(cors.Default()) } - authorized := r.Group("/", func(c *gin.Context) { - if !cfg.LocalAuth && isLocalRequest(c) { - return - } + // The device control plane below always requires a login session. These two + // settings used to open it without one; they no longer do. + if cfg.Password == "" { + log.Warn().Msg("password is empty: the built-in admin cannot log in; set 'password' (RTTYS_PASS)") + } + if !cfg.LocalAuth { + log.Warn().Msg("local-auth=false is ignored: local requests also require a login session") + } + authorized := r.Group("/", func(c *gin.Context) { if !httpAuth(cfg, c) { c.AbortWithStatus(http.StatusUnauthorized) return @@ -212,6 +218,11 @@ func (srv *RttyServer) ListenAPI() error { }) authorized.GET("/connect/:devid", func(c *gin.Context) { + if !canAccessDevice(c, c.Param("devid")) { + forbidDeviceAccess(c) + return + } + if !callUserHookUrl(cfg, c) { c.Status(http.StatusForbidden) return @@ -238,6 +249,14 @@ func (srv *RttyServer) ListenAPI() error { }) authorized.POST("/cmd/:devid", func(c *gin.Context) { + if !canAccessDevice(c, c.Param("devid")) { + c.JSON(http.StatusForbidden, gin.H{ + "err": rttyCmdErrForbidden, + "msg": cmdErrMsg[rttyCmdErrForbidden], + }) + return + } + if !callUserHookUrl(cfg, c) { c.Status(http.StatusForbidden) return @@ -261,6 +280,11 @@ func (srv *RttyServer) ListenAPI() error { }) authorized.Any("/web/:devid/:proto/:addr/*path", func(c *gin.Context) { + if !canAccessDevice(c, c.Param("devid")) { + forbidDeviceAccess(c) + return + } + httpProxyRedirect(srv, c, "") }) @@ -400,21 +424,10 @@ func callUserHookUrl(cfg *xconfig.Config, c *gin.Context) bool { return true } -func isLocalRequest(c *gin.Context) bool { - addr, _ := net.ResolveTCPAddr("tcp", c.Request.RemoteAddr) - return addr.IP.IsLoopback() -} - -func httpAuth(cfg *xconfig.Config, c *gin.Context) bool { - if !cfg.LocalAuth && isLocalRequest(c) { - return true - } - - // Keep legacy behavior: if password is not set, no auth required - if cfg.Password == "" { - return true - } - +// httpAuth gates the device control plane (/connect, /cmd, /web): a valid login +// session is always required. The legacy "empty password" and "local-auth off + +// loopback" shortcuts were removed -- they skipped authentication entirely. +func httpAuth(_ *xconfig.Config, c *gin.Context) bool { sid, err := c.Cookie("sid") if err != nil || strings.TrimSpace(sid) == "" { return false @@ -426,32 +439,98 @@ func httpAuth(cfg *xconfig.Config, c *gin.Context) bool { return ok } +// userIDFromCtx returns the logged-in user's id from the session cookie, or 0 +// when no session can be resolved. Session state is in memory, so no DB lookup. +func userIDFromCtx(c *gin.Context) int64 { + if c == nil || c.Request == nil || sessionStore == nil { + return 0 + } + sid, err := c.Cookie("sid") + if err != nil { + return 0 + } + sid = strings.TrimSpace(sid) + if sid == "" { + return 0 + } + sess, ok := sessionStore.Get(sid) + if !ok { + return 0 + } + return sess.UserID +} + // principalFromCtx best-effort extracts the logged-in user (id + username) // from the request, used for tagging device-event logs. Returns (0, "") // when no session can be resolved. Never blocks the calling path. func principalFromCtx(c *gin.Context) (int64, string) { - if c == nil || c.Request == nil || sessionStore == nil { - return 0, "" - } - sid, err := c.Cookie("sid") - if err != nil { - return 0, "" - } - sid = strings.TrimSpace(sid) - if sid == "" { - return 0, "" - } - sess, ok := sessionStore.Get(sid) - if !ok { + uid := userIDFromCtx(c) + if uid == 0 { return 0, "" } cont := sqlite.TryContainer() if cont == nil || cont.UserSvc == nil { - return sess.UserID, "" + return uid, "" } - u, err := cont.UserSvc.FindByID(c.Request.Context(), sess.UserID) + u, err := cont.UserSvc.FindByID(c.Request.Context(), uid) if err != nil || u == nil { - return sess.UserID, "" + return uid, "" } - return sess.UserID, u.Username + return uid, u.Username +} + +// canAccessDevice reports whether the logged-in user may use the device control +// endpoints (/connect, /cmd, /web) on devid. httpAuth only proves a session +// exists; this applies the same visibility rule as the device list: +// - admin -> every device +// - regular user -> only devices in a device group linked to one of its user +// groups (user_group_members -> user_group_device_group_links -> devices) +// +// Ungrouped devices therefore stay admin-only. Fails closed on any error. +func canAccessDevice(c *gin.Context, devid string) bool { + devid = strings.TrimSpace(devid) + if devid == "" { + return false + } + + uid := userIDFromCtx(c) + if uid == 0 { + return false + } + + cont := sqlite.TryContainer() + if cont == nil || cont.Gorm == nil || cont.UserSvc == nil { + return false + } + + // GetByID rejects disabled accounts, same as the /api auth middleware. + u, err := cont.UserSvc.GetByID(c.Request.Context(), uid) + if err != nil || u == nil { + return false + } + if u.Role == identity.RoleAdmin { + return true + } + + var n int64 + err = cont.Gorm.WithContext(c.Request.Context()).Raw( + `SELECT COUNT(1) + FROM devices d + JOIN user_group_device_group_links l ON l.device_group_id = d.device_group_id + JOIN user_group_members m ON m.group_id = l.user_group_id + WHERE d.ddns = ? AND m.user_id = ?`, + devid, uid, + ).Scan(&n).Error + return err == nil && n > 0 +} + +// forbidDeviceAccess rejects a control request for a device the caller may not +// access: browser navigations get the HTML error page, WebSocket upgrades a bare 403. +func forbidDeviceAccess(c *gin.Context) { + if c.GetHeader("Upgrade") == "websocket" { + c.AbortWithStatus(http.StatusForbidden) + return + } + c.Data(http.StatusForbidden, "text/html; charset=utf-8", []byte(generateErrorHTML("unauthorized"))) + c.Abort() } diff --git a/internal/server/command.go b/internal/server/command.go index a8fd309..29bca4a 100644 --- a/internal/server/command.go +++ b/internal/server/command.go @@ -61,15 +61,17 @@ type CommandRespInfo struct { } const ( - rttyCmdErrInvalid = 1001 - rttyCmdErrOffline = 1002 - rttyCmdErrTimeout = 1003 + rttyCmdErrInvalid = 1001 + rttyCmdErrOffline = 1002 + rttyCmdErrTimeout = 1003 + rttyCmdErrForbidden = 1004 ) var cmdErrMsg = map[int]string{ - rttyCmdErrInvalid: "invalid format", - rttyCmdErrOffline: "device offline", - rttyCmdErrTimeout: "timeout", + rttyCmdErrInvalid: "invalid format", + rttyCmdErrOffline: "device offline", + rttyCmdErrTimeout: "timeout", + rttyCmdErrForbidden: "permission denied", } func (dev *Device) handleCmdReq(c *gin.Context, info *CommandReqInfo) {