Ver Fonte

deactivate endpoints unless admin token set (#3910)

Alex Meijer há 1 mês atrás
pai
commit
a49a25bc2e
3 ficheiros alterados com 54 adições e 27 exclusões
  1. 15 0
      AGENTS.md
  2. 7 6
      pkg/costmodel/router.go
  3. 32 21
      pkg/costmodel/router_test.go

+ 15 - 0
AGENTS.md

@@ -147,6 +147,21 @@ just validate-protobuf
 | `MCP_SERVER_ENABLED` | `false` | Enable MCP server |
 | `MCP_HTTP_PORT` | `8081` | MCP server HTTP port |
 
+### Admin Auth
+
+| Variable | Default | Description |
+|----------|---------|-------------|
+| `ADMIN_TOKEN` | (unset) | Bearer token for admin endpoints. If unset, those endpoints return HTTP 503. |
+
+Admin-protected endpoints:
+
+- `POST /serviceKey`
+- `GET /helmValues`
+- `GET /cloudCost/rebuild`, `GET /cloudCost/repair`
+- `GET /cloud/config/export`, `GET /cloud/config/enable`, `GET /cloud/config/disable`, `GET /cloud/config/delete`
+
+**Breaking change:** deployments without `ADMIN_TOKEN` can no longer call these endpoints. Set `ADMIN_TOKEN` (for example via [opencost-helm-chart](https://github.com/opencost/opencost-helm-chart) `opencost.exporter.adminToken`) before using admin APIs.
+
 ### Cloud Providers
 
 | Variable | Description |

+ 7 - 6
pkg/costmodel/router.go

@@ -131,15 +131,16 @@ func ParsePercentString(percentStr string) (float64, error) {
 	return discount, nil
 }
 
-// adminAuthMiddleware wraps a handler and requires a Bearer token matching ADMIN_TOKEN env var when set.
-// When ADMIN_TOKEN is not set, logs a deduped warning and allows the request through.
-// When ADMIN_TOKEN is set, returns 401 if the Bearer token is missing or 403 if it does not match.
+// adminAuthMiddleware wraps a handler and requires a Bearer token matching ADMIN_TOKEN.
+// When ADMIN_TOKEN is not set, returns 503 with Cache-Control: no-store — the endpoint is
+// disabled until configured. When ADMIN_TOKEN is set, returns 401 if the Bearer token is
+// missing or 403 if it does not match.
 func adminAuthMiddleware(next httprouter.Handle) httprouter.Handle {
 	return func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 		adminToken := env.GetAdminToken()
 		if adminToken == "" {
-			log.DedupedWarningf(5, "Admin token (ADMIN_TOKEN) not configured; write operations are unauthenticated")
-			next(w, r, ps)
+			w.Header().Set("Cache-Control", "no-store")
+			http.Error(w, "Admin token is required to activate this endpoint; set the ADMIN_TOKEN environment variable", http.StatusServiceUnavailable)
 			return
 		}
 		authHeader := r.Header.Get("Authorization")
@@ -591,7 +592,7 @@ func Initialize(router *httprouter.Router, additionalConfigWatchers ...*watcher.
 	router.GET("/installNamespace", a.GetInstallNamespace)
 	router.GET("/installInfo", a.GetInstallInfo)
 	router.POST("/serviceKey", adminAuthMiddleware(a.AddServiceKey))
-	router.GET("/helmValues", a.GetHelmValues)
+	router.GET("/helmValues", adminAuthMiddleware(a.GetHelmValues))
 
 	return a
 }

+ 32 - 21
pkg/costmodel/router_test.go

@@ -3,7 +3,7 @@ package costmodel
 import (
 	"net/http"
 	"net/http/httptest"
-	"os"
+	"strings"
 	"testing"
 
 	"github.com/julienschmidt/httprouter"
@@ -20,18 +20,31 @@ func TestAdminAuthMiddleware(t *testing.T) {
 	}
 
 	tests := []struct {
-		name           string
-		setToken       string
-		authHeader     string
-		wantStatus     int
-		wantNextCalled bool
+		name              string
+		setToken          string
+		authHeader        string
+		wantStatus        int
+		wantNextCalled    bool
+		wantBodySubstr    string
+		wantCacheControl  string
 	}{
 		{
-			name:           "no admin token configured - request allowed with deduped warning",
-			setToken:       "",
-			authHeader:     "",
-			wantStatus:     http.StatusOK,
-			wantNextCalled: true,
+			name:             "no admin token configured - returns 503",
+			setToken:         "",
+			authHeader:       "",
+			wantStatus:       http.StatusServiceUnavailable,
+			wantNextCalled:   false,
+			wantBodySubstr:   "Admin token is required to activate this endpoint",
+			wantCacheControl: "no-store",
+		},
+		{
+			name:             "no admin token configured - bearer ignored, still 503",
+			setToken:         "",
+			authHeader:       "Bearer anything",
+			wantStatus:       http.StatusServiceUnavailable,
+			wantNextCalled:   false,
+			wantBodySubstr:   "Admin token is required to activate this endpoint",
+			wantCacheControl: "no-store",
 		},
 		{
 			name:           "missing authorization header",
@@ -71,18 +84,10 @@ func TestAdminAuthMiddleware(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			prev := os.Getenv(env.AdminTokenEnvVar)
-			defer func() {
-				if prev == "" {
-					os.Unsetenv(env.AdminTokenEnvVar)
-				} else {
-					os.Setenv(env.AdminTokenEnvVar, prev)
-				}
-			}()
 			if tt.setToken != "" {
-				os.Setenv(env.AdminTokenEnvVar, tt.setToken)
+				t.Setenv(env.AdminTokenEnvVar, tt.setToken)
 			} else {
-				os.Unsetenv(env.AdminTokenEnvVar)
+				t.Setenv(env.AdminTokenEnvVar, "")
 			}
 
 			nextCalled = false
@@ -101,6 +106,12 @@ func TestAdminAuthMiddleware(t *testing.T) {
 			if nextCalled != tt.wantNextCalled {
 				t.Errorf("nextCalled = %v, want %v", nextCalled, tt.wantNextCalled)
 			}
+			if tt.wantBodySubstr != "" && !strings.Contains(rec.Body.String(), tt.wantBodySubstr) {
+				t.Errorf("body = %q, want substring %q", rec.Body.String(), tt.wantBodySubstr)
+			}
+			if tt.wantCacheControl != "" && rec.Header().Get("Cache-Control") != tt.wantCacheControl {
+				t.Errorf("Cache-Control = %q, want %q", rec.Header().Get("Cache-Control"), tt.wantCacheControl)
+			}
 		})
 	}
 }