fix(display): allow browser sessions to take control #1832

Merged
agent-dev-b merged 1 commits from fix/display-control-browser-session into main 2026-05-25 04:24:38 +00:00
5 changed files with 110 additions and 5 deletions
+7 -3
View File
@@ -265,6 +265,11 @@ function DisplayControlBar({
onAcquire: () => void;
onRelease: () => void;
}) {
const userControl = control?.controller === "user";
const adminControl = userControl && control?.controlled_by === "admin-token";
const canAcquireUserControl = control?.controller === "none" || (userControl && !hasSession);
const canReleaseUserControl = adminControl || (userControl && hasSession);
return (
<div className="flex min-w-0 items-center gap-3">
{control && (
@@ -282,8 +287,7 @@ function DisplayControlBar({
{controlError && <p className="mt-0.5 text-[10px] text-red-200">{controlError}</p>}
</div>
)}
{(control?.controller === "none" ||
(control?.controller === "user" && control.controlled_by === "admin-token" && !hasSession)) && (
{canAcquireUserControl && (
<button
type="button"
onClick={onAcquire}
@@ -293,7 +297,7 @@ function DisplayControlBar({
Take control
</button>
)}
{control?.controller === "user" && control.controlled_by === "admin-token" && (
{canReleaseUserControl && (
<button
type="button"
onClick={onRelease}
@@ -337,11 +337,14 @@ func displayControlActor(c *gin.Context) (string, bool) {
return actorOrgTokenPrefix + s, true
}
}
if v, ok := c.Get("cp_session_actor"); ok {
if s, ok := v.(string); ok && s != "" {
return s, true
}
}
if displayControlIsAdminToken(c) {
return actorAdminToken, true
}
// Browser session auth is intentionally observe-only until AdminAuth
// exposes a stable per-user or per-session identity in gin.Context.
return "", false
}
@@ -258,6 +258,48 @@ func TestWorkspaceDisplayControlAcquire_RejectsCoarseSessionActor(t *testing.T)
}
}
func TestWorkspaceDisplayControlAcquire_AcceptsVerifiedBrowserSessionActor(t *testing.T) {
mock := setupTestDB(t)
t.Setenv("DISPLAY_SESSION_SIGNING_SECRET", "display-session-test-secret")
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
expiresAt := time.Date(2026, 5, 23, 18, 30, 0, 0, time.UTC)
mock.ExpectQuery(`SELECT COALESCE\(compute, '\{\}'::jsonb\) FROM workspaces WHERE id = \$1`).
WithArgs("ws-display").
WillReturnRows(sqlmock.NewRows([]string{"compute"}).AddRow(`{"display":{"mode":"desktop-control","protocol":"dcv","width":1920,"height":1080}}`))
mock.ExpectQuery(`INSERT INTO workspace_display_control_locks`).
WithArgs("ws-display", "user", "session:abc123", 300).
WillReturnRows(sqlmock.NewRows([]string{"controller", "controlled_by", "expires_at"}).
AddRow("user", "session:abc123", expiresAt))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-display"}}
c.Request = httptest.NewRequest("POST", "/workspaces/ws-display/display/control/acquire", bytes.NewBufferString(`{"controller":"user","ttl_seconds":300}`))
c.Request.Header.Set("Content-Type", "application/json")
c.Set("cp_session_actor", "session:abc123")
handler.AcquireDisplayControl(c)
if w.Code != http.StatusOK {
t.Fatalf("expected status 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["controller"] != "user" || resp["controlled_by"] != "session:abc123" {
t.Fatalf("lock response = %#v, want user/session actor", resp)
}
sessionURL, ok := resp["session_url"].(string)
if !ok || !strings.HasPrefix(sessionURL, "/workspaces/ws-display/display/session/websockify#token=") {
t.Fatalf("session_url = %#v, want signed websockify URL fragment", resp["session_url"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestWorkspaceDisplayControlRelease_RemovesCallerLock(t *testing.T) {
mock := setupTestDB(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
@@ -1,8 +1,10 @@
package middleware
import (
"crypto/sha256"
"crypto/subtle"
"database/sql"
"encoding/hex"
"errors"
"log"
"net/http"
@@ -196,6 +198,7 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
// bearer-only path unchanged.
if cookieHeader := c.GetHeader("Cookie"); cookieHeader != "" {
if ok, _ := VerifiedCPSession(cookieHeader); ok {
c.Set("cp_session_actor", cpSessionActor(cookieHeader))
c.Next()
return
}
@@ -260,6 +263,11 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
}
}
func cpSessionActor(cookieHeader string) string {
sum := sha256.Sum256([]byte(tenantSlug() + "\x00" + cookieHeader))
return "session:" + hex.EncodeToString(sum[:])[:16]
}
// CanvasOrBearer is a softer admin-auth variant used ONLY for cosmetic
// canvas routes where forging the request has zero security impact (PUT
// /canvas/viewport: worst case an attacker resets the shared viewport
@@ -284,6 +284,54 @@ func TestAdminAuth_FailOpen_NoTokensGlobally(t *testing.T) {
}
}
func TestAdminAuth_VerifiedCPSession_SetsSessionActor(t *testing.T) {
resetSessionCache()
srv, hits := mockCPServer(t, http.StatusOK, `{"member":true,"user_id":"u_1","role":"owner","org_id":"org_1"}`)
t.Setenv("CP_UPSTREAM_URL", srv.URL)
t.Setenv("MOLECULE_ORG_SLUG", "acme")
t.Setenv("ADMIN_TOKEN", "admin-secret")
mockDB, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("sqlmock.New: %v", err)
}
defer mockDB.Close()
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
const cookie = "session=valid-browser-session"
r := gin.New()
r.GET("/workspaces", AdminAuth(mockDB), func(c *gin.Context) {
actor, ok := c.Get("cp_session_actor")
if !ok {
t.Fatalf("expected cp_session_actor in context")
}
if actor != cpSessionActor(cookie) {
t.Fatalf("cp_session_actor = %q, want stable hashed actor", actor)
}
if actor == cookie {
t.Fatalf("cp_session_actor must not expose the raw cookie")
}
c.JSON(http.StatusOK, gin.H{"ok": true})
})
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodGet, "/workspaces", nil)
req.Header.Set("Cookie", cookie)
r.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200 for verified CP session, got %d: %s", w.Code, w.Body.String())
}
if hits.Load() != 1 {
t.Fatalf("expected one CP verification request, got %d", hits.Load())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestAdminAuth_C10_NoBearer_Returns401 — C10 critical path: when at least
// one workspace has tokens, GET /admin/secrets without a bearer → 401.
func TestAdminAuth_C10_NoBearer_Returns401(t *testing.T) {