test: cover device access checks on the control routes

Unit tests for canAccessDevice and httpAuth, an API test for script-info, and a live e2e case that registers two throwaway devices (one in the user's group, one not) and probes connect/cmd/web from an admin, a regular user and an anonymous caller. Signed-off-by: GL.iNet-Yongping.Xie <yongping.xie@gl-inet.com>
2026-10-14 16:30:07 +00:00 · 2026-10-14 00:38:52 -07:00
parent 111ef8ea33
commit 91f6b29147
3 changed files with 432 additions and 0 deletions
@@ -0,0 +1,86 @@
+package http
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"rttys/internal/domain/device"
+	"rttys/internal/domain/devicelog"
+	"rttys/internal/domain/notification"
+	"rttys/internal/domain/permission"
+	"rttys/internal/domain/user"
+	"rttys/internal/http/dto"
+	"rttys/internal/http/middleware"
+	"rttys/internal/store/memory"
+	"rttys/internal/store/sqlite"
+	"rttys/xconfig"
+
+	"github.com/gin-gonic/gin"
+)
+
+// script-info returns the device token and TURN credentials, so only roles with
+// device.write (admin) may read it.
+func TestScriptInfoRequiresDeviceWrite(t *testing.T) {
+	ctx := context.Background()
+	db, err := sqlite.Open(ctx, sqlite.Options{DSN: filepath.Join(t.TempDir(), "si.db")})
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	defer db.Close()
+	if err := sqlite.InitSchema(ctx, db.SQL(), "../../docker-compose/database/schema.sql"); err != nil {
+		t.Fatalf("init schema: %v", err)
+	}
+	g := db.Gorm()
+	if err := g.Exec(`INSERT INTO users (id, username, password_hash, role, status) VALUES
+		(1, 'admin', 'x', 'admin', 'active'), (2, 'alice', 'x', 'user', 'active')`).Error; err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	store := memory.NewSessionStore(time.Hour)
+	store.Create("sid-admin", 1)
+	store.Create("sid-alice", 2)
+
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	r.Use(middleware.Trace())
+	groupRepo := sqlite.NewGroupRepo(g)
+	RegisterAPIRoutes(r, Deps{
+		UserSvc:           user.NewService(sqlite.NewUserRepo(g)),
+		PermSvc:           permission.NewService(memory.NewPermissionRepo()),
+		DevSvc:            device.NewService(sqlite.NewDeviceRepo(g), groupRepo),
+		GroupRepo:         groupRepo,
+		SessionStore:      store,
+		RelationsRepo:     sqlite.NewRelationsRepo(g),
+		TrustedDeviceRepo: sqlite.NewTrustedDeviceRepo(g),
+		DeviceLogSvc:      devicelog.NewService(sqlite.NewDeviceLogRepo(g)),
+		NotificationSvc:   notification.NewService(sqlite.NewNotificationRepo(g)),
+		Cfg:               &xconfig.Config{Token: "device-token"},
+	})
+
+	get := func(sid string) (string, map[string]any) {
+		w := httptest.NewRecorder()
+		req := httptest.NewRequest(http.MethodGet, "/api/script-info", nil)
+		req.AddCookie(&http.Cookie{Name: "sid", Value: sid})
+		r.ServeHTTP(w, req)
+		var body struct {
+			Code string         `json:"code"`
+			Data map[string]any `json:"data"`
+		}
+		if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
+			t.Fatalf("decode %q: %v", w.Body.String(), err)
+		}
+		return body.Code, body.Data
+	}
+
+	if code, data := get("sid-alice"); code != dto.CodeForbidden || data["token"] != nil {
+		t.Errorf("regular user: code=%q token=%v, want %q and no token", code, data["token"], dto.CodeForbidden)
+	}
+	if code, data := get("sid-admin"); code != dto.CodeOK || data["token"] != "device-token" {
+		t.Errorf("admin: code=%q token=%v, want %q and the device token", code, data["token"], dto.CodeOK)
+	}
+}
@@ -0,0 +1,197 @@
+package qa
+
+import (
+	"bytes"
+	"crypto/tls"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"testing"
+	"time"
+)
+
+// Device CONTROL plane authorization (/connect, /cmd, /web) and credential
+// exposure via /api/script-info.
+//
+// A logged-in session alone must NOT be enough to drive a device: the caller
+// also has to be allowed to see that specific device. The device LIST already
+// enforced this (covered by TestE2E_GroupsRBAC); these endpoints did not, so a
+// regular user could reach any device — including ungrouped ones — by id, and
+// any logged-in user could read the device token and TURN credentials. This
+// suite pins both boundaries so they cannot regress.
+//
+// Run (same env vars as the rest of the qa suite):
+//
+//	QA_TARGET=https://<host> QA_USER=admin QA_PASS='<password>' \
+//	QA_DEV_ADDR=<host>:5912 QA_DEV_TOKEN='<rtty token>' \
+//	go test ./internal/qa -run TestE2E_DeviceControlAuthz -v
+
+// ctrl issues a request to a device control endpoint and returns the status
+// code. These routes live outside /api and authenticate via the `sid` cookie
+// (not the Bearer header), and they answer with redirects on success, so
+// redirects are left unfollowed and the status code is the assertion.
+func (c *client) ctrl(t *testing.T, method, path string, body any) int {
+	t.Helper()
+	var rdr io.Reader
+	if body != nil {
+		b, _ := json.Marshal(body)
+		rdr = bytes.NewReader(b)
+	}
+	req, err := http.NewRequest(method, c.cfg.target+path, rdr)
+	if err != nil {
+		t.Fatalf("ctrl %s %s: %v", method, path, err)
+	}
+	if body != nil {
+		req.Header.Set("Content-Type", "application/json")
+	}
+	if c.token != "" {
+		req.AddCookie(&http.Cookie{Name: "sid", Value: c.token})
+	}
+	hc := &http.Client{
+		Timeout:       15 * time.Second,
+		Transport:     &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}},
+		CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
+	}
+	resp, err := hc.Do(req)
+	if err != nil {
+		t.Fatalf("ctrl %s %s: %v", method, path, err)
+	}
+	defer resp.Body.Close()
+	io.Copy(io.Discard, resp.Body)
+	return resp.StatusCode
+}
+
+// controlProbes returns the three control endpoints for devid. The /cmd probe
+// deliberately carries an empty body: authorization is checked before the body
+// is parsed, so an allowed caller gets "invalid format" back and no command is
+// ever dispatched to the device.
+func controlProbes(devid string) []struct{ name, method, path string } {
+	return []struct{ name, method, path string }{
+		{"connect", http.MethodGet, "/connect/" + devid},
+		{"cmd", http.MethodPost, "/cmd/" + devid},
+		{"web", http.MethodGet, "/web/" + devid + "/http/127.0.0.1:80/"},
+	}
+}
+
+func TestE2E_DeviceControlAuthz(t *testing.T) {
+	cfg := loadConfig(t)
+	if cfg.pass == "" {
+		t.Skip("QA_PASS required")
+	}
+	if cfg.devAddr == "" {
+		t.Skip("QA_DEV_ADDR required to register fixture devices")
+	}
+
+	admin := newClient(cfg)
+	admin.login(t)
+
+	admin.cleanupQADevices(t)
+	defer admin.cleanupQADevices(t)
+
+	dgID := admin.createDeviceGroup(t, "qae2e_authz_dg")
+	ugID := admin.createUserGroup(t, "qae2e_authz_ug")
+	admin.linkUGtoDG(t, ugID, []int64{dgID})
+	uname, upass := "qae2e_authz_user", "Qae2ePass123!"
+	admin.createNormalUser(t, uname, upass, []int64{ugID})
+	defer func() {
+		if uid := admin.findUserID(t, uname); uid > 0 {
+			admin.del(t, fmt.Sprintf("/api/users/%d", uid))
+		}
+		admin.del(t, fmt.Sprintf("/api/user-groups/%d", ugID))
+		admin.del(t, fmt.Sprintf("/api/device-groups/%d", dgID))
+	}()
+
+	// Two throwaway devices: one the user is entitled to, one they are not.
+	// The out-of-scope device is left ungrouped, which is the case the device
+	// list never showed them and the control endpoints used to allow anyway.
+	inConn, inID := admin.registerQAOnline(t, "qae2e_authz_in", "02ffqaeauz01")
+	defer inConn.Close()
+	outConn, _ := admin.registerQAOnline(t, "qae2e_authz_out", "02ffqaeauz02")
+	defer outConn.Close()
+	admin.assignDevicesToGroup(t, dgID, []int64{inID})
+
+	user := loginClient(t, cfg, uname, upass)
+	if user == nil {
+		t.Fatal("normal user login failed")
+	}
+
+	// Sanity: the fixture really is in/out of scope on the management plane.
+	vl := user.listDevices(t, "pageSize=200")
+	if findItem(vl.Items, "qae2e_authz_in") == nil {
+		t.Fatal("fixture broken: user cannot see their own group's device")
+	}
+	if findItem(vl.Items, "qae2e_authz_out") != nil {
+		t.Fatal("fixture broken: user can see the out-of-scope device")
+	}
+
+	t.Run("in_scope_device_allowed", func(t *testing.T) {
+		for _, p := range controlProbes("qae2e_authz_in") {
+			if got := user.ctrl(t, p.method, p.path, nil); got == http.StatusForbidden {
+				t.Errorf("%s: user was denied their OWN group's device (403) — fix is too strict", p.name)
+			} else {
+				t.Logf("%s: http=%d (allowed)", p.name, got)
+			}
+		}
+	})
+
+	t.Run("out_of_scope_device_denied", func(t *testing.T) {
+		for _, p := range controlProbes("qae2e_authz_out") {
+			got := user.ctrl(t, p.method, p.path, nil)
+			if got != http.StatusForbidden {
+				t.Errorf("%s: out-of-scope device returned http=%d, want 403", p.name, got)
+			} else {
+				t.Logf("%s: http=403 (denied)", p.name)
+			}
+		}
+	})
+
+	t.Run("unknown_device_denied", func(t *testing.T) {
+		for _, p := range controlProbes("qae2e_authz_nosuchdev") {
+			if got := user.ctrl(t, p.method, p.path, nil); got != http.StatusForbidden {
+				t.Errorf("%s: unknown device returned http=%d, want 403", p.name, got)
+			}
+		}
+	})
+
+	t.Run("admin_reaches_both", func(t *testing.T) {
+		for _, devid := range []string{"qae2e_authz_in", "qae2e_authz_out"} {
+			for _, p := range controlProbes(devid) {
+				if got := admin.ctrl(t, p.method, p.path, nil); got == http.StatusForbidden {
+					t.Errorf("%s on %s: admin was denied (403) — fix broke admin access", p.name, devid)
+				}
+			}
+		}
+	})
+
+	t.Run("no_session_denied", func(t *testing.T) {
+		anon := newClient(cfg) // no token => no sid cookie
+		for _, p := range controlProbes("qae2e_authz_in") {
+			if got := anon.ctrl(t, p.method, p.path, nil); got != http.StatusUnauthorized {
+				t.Errorf("%s: anonymous request returned http=%d, want 401", p.name, got)
+			}
+		}
+	})
+
+	// /api/script-info hands out the device registration token and the TURN
+	// credentials, so it must be restricted to roles that may add devices.
+	t.Run("script_info_admin_only", func(t *testing.T) {
+		_, env, err := user.do("GET", "/api/script-info", nil, true)
+		if err != nil {
+			t.Fatalf("script-info as user: %v", err)
+		}
+		if env.OK {
+			t.Errorf("script-info: regular user got code=%s, want a denial", env.Code)
+		} else {
+			t.Logf("script-info as user: code=%s (denied)", env.Code)
+		}
+
+		_, env, err = admin.do("GET", "/api/script-info", nil, true)
+		if err != nil {
+			t.Fatalf("script-info as admin: %v", err)
+		}
+		if !env.OK {
+			t.Errorf("script-info: admin got code=%s, want it to still work", env.Code)
+		}
+	})
+}
@@ -0,0 +1,149 @@
+package server
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"rttys/internal/domain/user"
+	"rttys/internal/store/memory"
+	"rttys/internal/store/sqlite"
+	"rttys/xconfig"
+
+	"github.com/gin-gonic/gin"
+)
+
+// setupAccessFixture installs a temp DB as the global container plus a fresh
+// session store, with one session per user (sid "sid-<username>"):
+//
+//	admin (admin)       alice (user, ug1)    bob (user, no group)    carol (user, ug1, disabled)
+//	ug1 -> dg1          dev-g1 in dg1        dev-g2 in dg2 (not linked)    dev-none ungrouped
+func setupAccessFixture(t *testing.T) {
+	t.Helper()
+	ctx := context.Background()
+
+	db, err := sqlite.Open(ctx, sqlite.Options{DSN: filepath.Join(t.TempDir(), "access.db")})
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	if err := sqlite.InitSchema(ctx, db.SQL(), "../../docker-compose/database/schema.sql"); err != nil {
+		t.Fatalf("init schema: %v", err)
+	}
+	g := db.Gorm()
+
+	stmts := []string{
+		`INSERT INTO users (id, username, password_hash, role, status) VALUES
+			(1, 'admin', 'x', 'admin', 'active'),
+			(2, 'alice', 'x', 'user',  'active'),
+			(3, 'bob',   'x', 'user',  'active'),
+			(4, 'carol', 'x', 'user',  'disabled')`,
+		`INSERT INTO user_groups (id, name) VALUES (1, 'ug1')`,
+		`INSERT INTO user_group_members (user_id, group_id) VALUES (2, 1), (4, 1)`,
+		`INSERT INTO device_groups (id, name) VALUES (1, 'dg1'), (2, 'dg2')`,
+		`INSERT INTO user_group_device_group_links (user_group_id, device_group_id) VALUES (1, 1)`,
+		`INSERT INTO devices (ddns, mac, device_group_id) VALUES
+			('dev-g1',   'aa0000000001', 1),
+			('dev-g2',   'aa0000000002', 2),
+			('dev-none', 'aa0000000003', NULL)`,
+	}
+	for _, s := range stmts {
+		if err := g.Exec(s).Error; err != nil {
+			t.Fatalf("seed: %v\n%s", err, s)
+		}
+	}
+
+	prevStore, prevCont := sessionStore, sqlite.TryContainer()
+	sessionStore = memory.NewSessionStore(time.Hour)
+	for id, name := range map[int64]string{1: "admin", 2: "alice", 3: "bob", 4: "carol"} {
+		sessionStore.Create("sid-"+name, id)
+	}
+	sqlite.SetContainer(&sqlite.Container{Gorm: g, UserSvc: user.NewService(sqlite.NewUserRepo(g))})
+
+	t.Cleanup(func() {
+		sessionStore = prevStore
+		sqlite.SetContainer(prevCont)
+		db.Close()
+	})
+}
+
+func ctxWithSid(sid string) (*gin.Context, *httptest.ResponseRecorder) {
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	req := httptest.NewRequest(http.MethodGet, "/connect/x", nil)
+	if sid != "" {
+		req.AddCookie(&http.Cookie{Name: "sid", Value: sid})
+	}
+	c.Request = req
+	return c, w
+}
+
+func TestCanAccessDevice(t *testing.T) {
+	setupAccessFixture(t)
+
+	cases := []struct {
+		sid, devid string
+		want       bool
+	}{
+		{"sid-admin", "dev-g1", true},
+		{"sid-admin", "dev-g2", true},
+		{"sid-admin", "dev-none", true}, // admin also reaches ungrouped devices
+		{"sid-alice", "dev-g1", true},   // in a device group linked to her user group
+		{"sid-alice", "dev-g2", false},  // device group not linked to her
+		{"sid-alice", "dev-none", false},
+		{"sid-alice", "dev-unknown", false},
+		{"sid-alice", " ", false},
+		{"sid-bob", "dev-g1", false},   // no user group at all
+		{"sid-carol", "dev-g1", false}, // disabled account
+		{"sid-bogus", "dev-g1", false}, // unknown session
+		{"", "dev-g1", false},          // no session
+	}
+	for _, tc := range cases {
+		c, _ := ctxWithSid(tc.sid)
+		if got := canAccessDevice(c, tc.devid); got != tc.want {
+			t.Errorf("canAccessDevice(sid=%q, devid=%q) = %v, want %v", tc.sid, tc.devid, got, tc.want)
+		}
+	}
+}
+
+func TestHttpAuthAlwaysRequiresSession(t *testing.T) {
+	setupAccessFixture(t)
+
+	// Settings that used to skip authentication must no longer do so.
+	cfg := &xconfig.Config{Password: "", LocalAuth: false}
+
+	c, _ := ctxWithSid("")
+	c.Request.RemoteAddr = "127.0.0.1:50000"
+	if httpAuth(cfg, c) {
+		t.Error("httpAuth allowed a loopback request without a session")
+	}
+
+	c, _ = ctxWithSid("sid-bogus")
+	if httpAuth(cfg, c) {
+		t.Error("httpAuth allowed an unknown session")
+	}
+
+	c, _ = ctxWithSid("sid-alice")
+	if !httpAuth(cfg, c) {
+		t.Error("httpAuth rejected a valid session")
+	}
+}
+
+func TestForbidDeviceAccess(t *testing.T) {
+	c, w := ctxWithSid("")
+	forbidDeviceAccess(c)
+	if w.Code != http.StatusForbidden || !strings.Contains(w.Body.String(), "<html") {
+		t.Errorf("page request: code=%d, want 403 with the HTML error page", w.Code)
+	}
+
+	c, w = ctxWithSid("")
+	c.Request.Header.Set("Upgrade", "websocket")
+	forbidDeviceAccess(c)
+	if w.Code != http.StatusForbidden || w.Body.Len() != 0 {
+		t.Errorf("websocket request: code=%d body=%d bytes, want bare 403", w.Code, w.Body.Len())
+	}
+}