refactor(hub): simplify universal token request parsing and use ApiScenario tests

2026-10-22 06:12:33 +00:00 · 2026-10-19 12:47:42 -04:00
parent 7b6d995533
commit 226590b2df
3 changed files with 275 additions and 141 deletions
@@ -193,9 +193,9 @@ func (h *Hub) registerApiRoutes(se *core.ServeEvent) error {
 	apiAuth.GET("/config-yaml", config.GetYamlConfig).BindFunc(requireAdminRole)
 	// handle agent websocket connection
 	apiNoAuth.GET("/agent-connect", h.handleAgentConnect)
-	// Preserve GET for existing clients; the dashboard sends tokens in POST bodies.
-	apiAuth.GET("/universal-token", h.getUniversalToken).BindFunc(excludeReadOnlyRole)
-	apiAuth.POST("/universal-token", h.getUniversalToken).BindFunc(excludeReadOnlyRole)
+	// get, create, or delete universal tokens (POST keeps tokens out of URLs; GET is kept for legacy clients)
+	apiAuth.GET("/universal-token", h.handleUniversalToken).BindFunc(excludeReadOnlyRole)
+	apiAuth.POST("/universal-token", h.handleUniversalToken).BindFunc(excludeReadOnlyRole)
 	// update / delete user alerts
 	apiAuth.POST("/user-alerts", alerts.UpsertUserAlerts)
 	apiAuth.DELETE("/user-alerts", alerts.DeleteUserAlerts)
@@ -261,36 +261,42 @@ func (info *UpdateInfo) getUpdate(e *core.RequestEvent) error {
 	return e.JSON(http.StatusOK, info)
 }

-// GetUniversalToken handles the universal token API endpoint (create, read, delete)
-func (h *Hub) getUniversalToken(e *core.RequestEvent) error {
+// universalTokenRequest holds the options for the universal token endpoint.
+// Enable and Permanent use 0 for false, 1 for true, and -1 for unset.
+type universalTokenRequest struct {
+	Token     string `json:"token"`
+	Enable    int    `json:"enable"`
+	Permanent int    `json:"permanent"`
+}
+
+// handleUniversalToken handles the universal token API endpoint (create, read, delete)
+func (h *Hub) handleUniversalToken(e *core.RequestEvent) error {
 	e.Response.Header().Set("Cache-Control", "no-store")
 	if e.Auth.IsSuperuser() {
 		return e.ForbiddenError("Superusers cannot use universal tokens", nil)
 	}

-	tokenMap := universalTokenMap.GetMap()
-	userID := e.Auth.Id
-	query := e.Request.URL.Query()
-	token := query.Get("token")
-	enable := query.Get("enable")
-	permanent := query.Get("permanent")
+	req := universalTokenRequest{Enable: -1, Permanent: -1}
 	if e.Request.Method == http.MethodPost {
-		data := struct {
-			Token     string `json:"token"`
-			Enable    int    `json:"enable"`
-			Permanent int    `json:"permanent"`
-		}{Enable: -1, Permanent: -1}
-		if err := e.BindBody(&data); err != nil {
+		if err := e.BindBody(&req); err != nil {
 			return e.BadRequestError("Invalid universal token request", err)
 		}
-		if data.Enable < -1 || data.Enable > 1 || data.Permanent < -1 || data.Permanent > 1 {
-			return e.BadRequestError("Invalid universal token options", nil)
+	} else {
+		// legacy GET requests pass options as query params
+		query := e.Request.URL.Query()
+		req.Token = query.Get("token")
+		if v, err := strconv.Atoi(query.Get("enable")); err == nil {
+			req.Enable = v
+		}
+		if v, err := strconv.Atoi(query.Get("permanent")); err == nil {
+			req.Permanent = v
 		}
-		token = data.Token
-		enable = strconv.Itoa(data.Enable)
-		permanent = strconv.Itoa(data.Permanent)
 	}

+	tokenMap := universalTokenMap.GetMap()
+	userID := e.Auth.Id
+	token := req.Token
+
 	// helper for deleting any existing permanent token record for this user
 	deletePermanent := func() error {
 		rec, err := h.FindFirstRecordByFilter("universal_tokens", "user = {:user}", dbx.Params{"user": userID})
@@ -319,19 +325,19 @@ func (h *Hub) getUniversalToken(e *core.RequestEvent) error {
 	}

 	// Disable universal tokens (both ephemeral and permanent)
-	if enable == "0" {
+	if req.Enable == 0 {
 		tokenMap.RemovebyValue(userID)
 		_ = deletePermanent()
 		return e.JSON(http.StatusOK, map[string]any{"token": token, "active": false, "permanent": false})
 	}

 	// Enable universal token (ephemeral or permanent)
-	if enable == "1" {
+	if req.Enable == 1 {
 		if token == "" {
 			token = uuid.New().String()
 		}

-		if permanent == "1" {
+		if req.Permanent == 1 {
 			// make token permanent (persist across restarts)
 			tokenMap.RemovebyValue(userID)
 			if err := upsertPermanent(token); err != nil {
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"sort"
+	"strings"
 	"testing"
 	"time"

@@ -814,6 +815,249 @@ func TestApiRoutesAuthentication(t *testing.T) {
 	}
 }

+func TestUniversalTokenEndpoint(t *testing.T) {
+	hub, user := beszelTests.GetHubWithUser(t)
+	defer hub.Cleanup()
+
+	userToken, err := user.NewAuthToken()
+	require.NoError(t, err, "Failed to create auth token")
+
+	otherUser, err := beszelTests.CreateUser(hub, "other@example.com", "password123")
+	require.NoError(t, err, "Failed to create other user")
+	otherUserToken, err := otherUser.NewAuthToken()
+	require.NoError(t, err, "Failed to create other user auth token")
+
+	readOnlyUser, err := beszelTests.CreateUserWithRole(hub, "readonly@example.com", "password123", "readonly")
+	require.NoError(t, err, "Failed to create readonly user")
+	readOnlyUserToken, err := readOnlyUser.NewAuthToken()
+	require.NoError(t, err, "Failed to create readonly user auth token")
+
+	superuser, err := beszelTests.CreateSuperuser(hub, "superuser@example.com", "password123")
+	require.NoError(t, err, "Failed to create superuser")
+	superuserToken, err := superuser.NewAuthToken()
+	require.NoError(t, err, "Failed to create superuser auth token")
+
+	testAppFactory := func(t testing.TB) *pbTests.TestApp {
+		return hub.TestApp
+	}
+
+	// token responses should never be cached
+	expectNoStore := func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+		require.Equal(t, "no-store", res.Header.Get("Cache-Control"))
+	}
+
+	// asserts whether the user has a permanent token record with the given token
+	expectPermanentRecord := func(userID, token string, exists bool) func(testing.TB, *pbTests.TestApp, *http.Response) {
+		return func(t testing.TB, app *pbTests.TestApp, res *http.Response) {
+			expectNoStore(t, app, res)
+			rec, err := app.FindFirstRecordByFilter("universal_tokens", "user = {:user}", dbx.Params{"user": userID})
+			if !exists {
+				require.Error(t, err, "Expected no permanent token record")
+				return
+			}
+			require.NoError(t, err, "Expected permanent token record")
+			require.Equal(t, token, rec.GetString("token"))
+		}
+	}
+
+	const path = "/api/beszel/universal-token"
+	const token = "post-universal-token-123"
+
+	// scenarios share app state and run in order
+	scenarios := []beszelTests.ApiScenario{
+		{
+			Name:            "POST /universal-token - no auth should fail",
+			Method:          http.MethodPost,
+			URL:             path,
+			Body:            jsonReader(map[string]any{"enable": 1}),
+			ExpectedStatus:  401,
+			ExpectedContent: []string{"requires valid"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /universal-token - readonly user should fail",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"enable": 1}),
+			Headers: map[string]string{
+				"Authorization": readOnlyUserToken,
+			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{"The authorized record is not allowed to perform this action."},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /universal-token - superuser should fail",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"enable": 1}),
+			Headers: map[string]string{
+				"Authorization": superuserToken,
+			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{"Superusers cannot use universal tokens"},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectNoStore,
+		},
+		{
+			Name:   "POST /universal-token - invalid json should fail",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   strings.NewReader(`{`),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  400,
+			ExpectedContent: []string{"Invalid universal token request"},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectNoStore,
+		},
+		{
+			Name:   "POST /universal-token - non-numeric enable should fail",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"enable": "yes"}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  400,
+			ExpectedContent: []string{"Invalid universal token request"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /universal-token - enable ephemeral token",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"token": token, "enable": 1}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"` + token + `"`, `"active":true`, `"permanent":false`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectPermanentRecord(user.Id, "", false),
+		},
+		{
+			Name:   "POST /universal-token - empty body should read current state",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"` + token + `"`, `"active":true`, `"permanent":false`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectNoStore,
+		},
+		{
+			Name:   "POST /universal-token - make token permanent",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"token": token, "enable": 1, "permanent": 1}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"` + token + `"`, `"active":true`, `"permanent":true`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectPermanentRecord(user.Id, token, true),
+		},
+		{
+			Name:   "POST /universal-token - other user should not read token",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{}),
+			Headers: map[string]string{
+				"Authorization": otherUserToken,
+			},
+			ExpectedStatus:     200,
+			ExpectedContent:    []string{`"active":false`},
+			NotExpectedContent: []string{token},
+			TestAppFactory:     testAppFactory,
+		},
+		{
+			Name:   "POST /universal-token - other user should see token as inactive",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"token": token}),
+			Headers: map[string]string{
+				"Authorization": otherUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"` + token + `"`, `"active":false`},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /universal-token - other user should not disable token",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"enable": 0}),
+			Headers: map[string]string{
+				"Authorization": otherUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"active":false`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectPermanentRecord(user.Id, token, true),
+		},
+		{
+			Name:   "POST /universal-token - disable permanent token",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{"enable": 0}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"active":false`, `"permanent":false`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectPermanentRecord(user.Id, "", false),
+		},
+		{
+			Name:   "POST /universal-token - read after disable should be inactive",
+			Method: http.MethodPost,
+			URL:    path,
+			Body:   jsonReader(map[string]any{}),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:     200,
+			ExpectedContent:    []string{`"active":false`, `"permanent":false`},
+			NotExpectedContent: []string{token},
+			TestAppFactory:     testAppFactory,
+		},
+		{
+			Name:   "GET /universal-token - legacy query params should still work",
+			Method: http.MethodGet,
+			URL:    path + "?enable=1&permanent=1&token=legacy-token-123",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"legacy-token-123"`, `"active":true`, `"permanent":true`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectPermanentRecord(user.Id, "legacy-token-123", true),
+		},
+		{
+			Name:   "GET /universal-token - legacy read without query params",
+			Method: http.MethodGet,
+			URL:    path,
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"token":"legacy-token-123"`, `"active":true`, `"permanent":true`},
+			TestAppFactory:  testAppFactory,
+			AfterTestFunc:   expectNoStore,
+		},
+	}
+
+	for _, scenario := range scenarios {
+		scenario.Test(t)
+	}
+}
+
 func TestFirstUserCreation(t *testing.T) {
 	t.Run("CreateUserEndpoint available when no users exist", func(t *testing.T) {
 		hub, _ := beszelTests.NewTestHub(t.TempDir())
@@ -1,116 +0,0 @@
-//go:build testing
-
-package hub_test
-
-import (
-	"encoding/json"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	beszelTests "github.com/henrygd/beszel/internal/tests"
-	"github.com/stretchr/testify/require"
-)
-
-func TestUniversalTokenJSONRequests(t *testing.T) {
-	hub, handler := firstUserTestMux(t)
-	defer hub.Cleanup()
-	user, err := beszelTests.CreateUser(hub, "token-user@example.com", "password123")
-	require.NoError(t, err)
-	userToken, err := user.NewAuthToken()
-	require.NoError(t, err)
-
-	request := func(t *testing.T, method, path, body, auth string) *httptest.ResponseRecorder {
-		t.Helper()
-		req := httptest.NewRequest(method, path, strings.NewReader(body))
-		req.Header.Set("Content-Type", "application/json")
-		if auth != "" {
-			req.Header.Set("Authorization", auth)
-		}
-		response := httptest.NewRecorder()
-		handler.ServeHTTP(response, req)
-		return response
-	}
-	decode := func(t *testing.T, response *httptest.ResponseRecorder) map[string]any {
-		t.Helper()
-		require.Equal(t, http.StatusOK, response.Code, response.Body.String())
-		require.Equal(t, "no-store", response.Header().Get("Cache-Control"))
-		var data map[string]any
-		require.NoError(t, json.Unmarshal(response.Body.Bytes(), &data))
-		return data
-	}
-	const path = "/api/beszel/universal-token"
-	const token = "dummy-universal-token-json-request"
-
-	t.Run("enable ephemeral token through the request body", func(t *testing.T) {
-		data := decode(t, request(t, http.MethodPost, path, `{"token":"`+token+`","enable":1}`, userToken))
-		require.Equal(t, token, data["token"])
-		require.Equal(t, true, data["active"])
-		require.Equal(t, false, data["permanent"])
-	})
-	t.Run("read current state without a token in the URL", func(t *testing.T) {
-		data := decode(t, request(t, http.MethodPost, path, `{}`, userToken))
-		require.Equal(t, token, data["token"])
-		require.Equal(t, true, data["active"])
-	})
-	t.Run("make the token permanent through the request body", func(t *testing.T) {
-		data := decode(t, request(t, http.MethodPost, path, `{"token":"`+token+`","enable":1,"permanent":1}`, userToken))
-		require.Equal(t, token, data["token"])
-		require.Equal(t, true, data["permanent"])
-	})
-	t.Run("preserve token ownership across users", func(t *testing.T) {
-		other, err := beszelTests.CreateUser(hub, "other-token-user@example.com", "password123")
-		require.NoError(t, err)
-		otherToken, err := other.NewAuthToken()
-		require.NoError(t, err)
-		data := decode(t, request(t, http.MethodPost, path, `{}`, otherToken))
-		require.NotEqual(t, token, data["token"])
-		data = decode(t, request(t, http.MethodPost, path, `{"token":"`+token+`"}`, otherToken))
-		require.Equal(t, false, data["active"])
-		decode(t, request(t, http.MethodPost, path, `{"enable":0}`, otherToken))
-		data = decode(t, request(t, http.MethodPost, path, `{}`, userToken))
-		require.Equal(t, token, data["token"])
-		require.Equal(t, true, data["active"])
-	})
-	t.Run("disable the permanent token through the request body", func(t *testing.T) {
-		data := decode(t, request(t, http.MethodPost, path, `{"enable":0}`, userToken))
-		require.Equal(t, false, data["active"])
-		data = decode(t, request(t, http.MethodPost, path, `{}`, userToken))
-		require.Equal(t, false, data["active"])
-		require.Equal(t, false, data["permanent"])
-	})
-	t.Run("keep legacy GET requests compatible and uncached", func(t *testing.T) {
-		data := decode(t, request(t, http.MethodGet, path+"?enable=1&permanent=1&token=legacy-dummy-token", "", userToken))
-		require.Equal(t, "legacy-dummy-token", data["token"])
-		require.Equal(t, true, data["active"])
-		data = decode(t, request(t, http.MethodGet, path, "", userToken))
-		require.Equal(t, "legacy-dummy-token", data["token"])
-	})
-
-	for _, body := range []string{`{`, `{"enable":"yes"}`, `{"enable":2}`, `{"permanent":2}`, `{"enable":-2}`} {
-		t.Run("reject invalid request "+body, func(t *testing.T) {
-			response := request(t, http.MethodPost, path, body, userToken)
-			require.Equal(t, http.StatusBadRequest, response.Code, response.Body.String())
-			require.Equal(t, "no-store", response.Header().Get("Cache-Control"))
-		})
-	}
-	t.Run("require authentication", func(t *testing.T) {
-		response := request(t, http.MethodPost, path, `{"enable":1}`, "")
-		require.Equal(t, http.StatusUnauthorized, response.Code)
-	})
-	t.Run("reject readonly users", func(t *testing.T) {
-		user, err := beszelTests.CreateUserWithRole(hub, "readonly-token@example.com", "password123", "readonly")
-		require.NoError(t, err)
-		auth, err := user.NewAuthToken()
-		require.NoError(t, err)
-		require.Equal(t, http.StatusForbidden, request(t, http.MethodPost, path, `{"enable":1}`, auth).Code)
-	})
-	t.Run("reject superusers", func(t *testing.T) {
-		user, err := beszelTests.CreateSuperuser(hub, "superuser-token@example.com", "password123")
-		require.NoError(t, err)
-		auth, err := user.NewAuthToken()
-		require.NoError(t, err)
-		require.Equal(t, http.StatusForbidden, request(t, http.MethodPost, path, `{"enable":1}`, auth).Code)
-	})
-}