fix(security): close auth bypass and nil-config holes in site proxy (#20, #27) (#39)

Issue #20 — authorization bypass for non-SA tokens:
The previous code only called authorize() when parseServiceAccountToken()
succeeded. Any other token (opaque, arbitrary JWT, malformed) fell through
the if-block without rejection and was silently proxied to the site cluster.
Fix: unrecognized token types now get an explicit 403 Forbidden.

Issue #27 — site proxy unauthenticated when kcpConfig is nil:
The entire auth block was guarded by 'if p.kcpConfig != nil', so a nil
kcpConfig (no kcp backend) allowed any non-empty bearer token through.
Fix: nil kcpConfig now returns 503 Service Unavailable immediately — the
site proxy cannot safely authorize requests without a kcp backend.

Also updated the NewVirtualWorkspaces doc comment which incorrectly stated
that nil kcpConfig is acceptable in dev mode.

Co-authored-by: Mangirdas Judeikis <mjudeikis@gmail.com>

Author: mjudeikis-bot

pkg/virtual/builder/proxy.go

@@ -65,8 +65,8 @@ type VirtualWorkspaceHandlers struct {
 }
 
 // NewVirtualWorkspaces creates a new VirtualWorkspaceHandlers.
-// kcpConfig is used for SA token verification against kcp. If nil, token
-// verification is skipped (dev mode only).
+// kcpConfig is required for SA token authorization against kcp. A nil
+// kcpConfig causes the site-proxy handler to reject all requests with 503.
 func NewVirtualWorkspaces(cm *connman.ConnectionManager, kcpConfig *rest.Config, siteRoutes *SiteRouteMap, logger klog.Logger) *VirtualWorkspaceHandlers {
 	return &VirtualWorkspaceHandlers{
 		vws: &virtualWorkspaces{

pkg/virtual/builder/site_proxy_handler.go

@@ -63,20 +63,38 @@ func (p *virtualWorkspaces) buildSiteProxyHandler() http.Handler {
 
 		logger.V(4).Info("Site proxy request", "cluster", clusterName, "site", siteName, "apiPath", apiPath)
 
-		// Auth: trust kcp admin bearer token (mount proxy), or validate SA token.
+		// Auth: kcp backend config is required for authorization.
+		// Refuse to serve any request when kcpConfig is nil — there is no way
+		// to validate tokens without a kcp backend (fix for #27).
+		if p.kcpConfig == nil {
+			logger.Error(nil, "site proxy has no kcp backend config; rejecting request")
+			http.Error(w, "Service Unavailable: site proxy not configured", http.StatusServiceUnavailable)
+			return
+		}
+
+		// Extract and validate the bearer token.
 		token := extractBearerToken(r)
 		if token == "" {
 			http.Error(w, "Unauthorized", http.StatusUnauthorized)
 			return
 		}
 
-		if p.kcpConfig != nil && token != p.kcpConfig.BearerToken {
-			if claims, ok := parseServiceAccountToken(token); ok {
-				if err := authorize(r.Context(), p.kcpConfig, token, claims.ClusterName, "proxy", "sites", siteName); err != nil {
-					logger.Error(err, "site proxy authorization failed", "site", siteName)
-					http.Error(w, "Forbidden", http.StatusForbidden)
-					return
-				}
+		// Trust the kcp admin bearer token (used by the mount proxy internally).
+		// For all other tokens, require a valid ServiceAccount token that passes
+		// kcp RBAC authorization. Unknown/opaque tokens are explicitly rejected
+		// rather than silently passed through (fix for #20).
+		if token != p.kcpConfig.BearerToken {
+			claims, ok := parseServiceAccountToken(token)
+			if !ok {
+				// Token is not a recognized ServiceAccount JWT — reject.
+				logger.Info("site proxy rejected unrecognized token type", "site", siteName)
+				http.Error(w, "Forbidden", http.StatusForbidden)
+				return
+			}
+			if err := authorize(r.Context(), p.kcpConfig, token, claims.ClusterName, "proxy", "sites", siteName); err != nil {
+				logger.Error(err, "site proxy authorization failed", "site", siteName)
+				http.Error(w, "Forbidden", http.StatusForbidden)
+				return
 			}
 		}