Skip to content

Commit 876c001

Browse files
authored
Merge pull request #56 from Bin-Huang/fix/protect-tasks-list
Protect tasks list endpoint
2 parents 1a63acc + 601987b commit 876c001

3 files changed

Lines changed: 172 additions & 10 deletions

File tree

internal/setup/handlers_skills_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515

1616
func TestListSkillsRequiresAuth(t *testing.T) {
1717
ctx := context.Background()
18-
s, resolver, adminUser, regularUser := newSkillsAuthTestServer(t, ctx)
18+
s, resolver, adminUser, regularUser := newAuthTestServer(t, ctx)
1919
t.Setenv("FASTCLAW_HOME", t.TempDir())
2020

2121
handler := s.authMiddleware(s.handleListSkills)
@@ -30,7 +30,7 @@ func TestListSkillsRequiresAuth(t *testing.T) {
3030

3131
t.Run("regular user is allowed", func(t *testing.T) {
3232
rr := httptest.NewRecorder()
33-
handler(rr, skillsListRequest(t, ctx, resolver, regularUser.ID))
33+
handler(rr, authTestRequest(t, ctx, resolver, http.MethodGet, "/api/skills", regularUser.ID))
3434
if rr.Code != http.StatusOK {
3535
t.Fatalf("status = %d, want %d", rr.Code, http.StatusOK)
3636
}
@@ -41,7 +41,7 @@ func TestListSkillsRequiresAuth(t *testing.T) {
4141

4242
t.Run("super admin is allowed", func(t *testing.T) {
4343
rr := httptest.NewRecorder()
44-
handler(rr, skillsListRequest(t, ctx, resolver, adminUser.ID))
44+
handler(rr, authTestRequest(t, ctx, resolver, http.MethodGet, "/api/skills", adminUser.ID))
4545
if rr.Code != http.StatusOK {
4646
t.Fatalf("status = %d, want %d", rr.Code, http.StatusOK)
4747
}
@@ -51,7 +51,7 @@ func TestListSkillsRequiresAuth(t *testing.T) {
5151
})
5252
}
5353

54-
func newSkillsAuthTestServer(t *testing.T, ctx context.Context) (*Server, *auth.Resolver, *users.Account, *users.Account) {
54+
func newAuthTestServer(t *testing.T, ctx context.Context) (*Server, *auth.Resolver, *users.Account, *users.Account) {
5555
t.Helper()
5656

5757
dbPath := filepath.Join(t.TempDir(), "fastclaw.db")
@@ -70,8 +70,8 @@ func newSkillsAuthTestServer(t *testing.T, ctx context.Context) (*Server, *auth.
7070
if err != nil {
7171
t.Fatalf("NewAccounts: %v", err)
7272
}
73-
adminUser := createSkillsTestUser(t, ctx, accts, "admin", users.RoleSuperAdmin)
74-
regularUser := createSkillsTestUser(t, ctx, accts, "user", users.RoleUser)
73+
adminUser := createAuthTestUser(t, ctx, accts, "admin", users.RoleSuperAdmin)
74+
regularUser := createAuthTestUser(t, ctx, accts, "user", users.RoleUser)
7575
resolver, err := auth.NewResolver(st)
7676
if err != nil {
7777
t.Fatalf("NewResolver: %v", err)
@@ -83,7 +83,7 @@ func newSkillsAuthTestServer(t *testing.T, ctx context.Context) (*Server, *auth.
8383
return s, resolver, adminUser, regularUser
8484
}
8585

86-
func createSkillsTestUser(t *testing.T, ctx context.Context, accts *users.Accounts, username, role string) *users.Account {
86+
func createAuthTestUser(t *testing.T, ctx context.Context, accts *users.Accounts, username, role string) *users.Account {
8787
t.Helper()
8888

8989
acct, err := accts.Create(ctx, users.CreateInput{
@@ -98,14 +98,14 @@ func createSkillsTestUser(t *testing.T, ctx context.Context, accts *users.Accoun
9898
return acct
9999
}
100100

101-
func skillsListRequest(t *testing.T, ctx context.Context, resolver *auth.Resolver, userID string) *http.Request {
101+
func authTestRequest(t *testing.T, ctx context.Context, resolver *auth.Resolver, method, path, userID string) *http.Request {
102102
t.Helper()
103103

104104
cookie, err := resolver.IssueSession(ctx, userID)
105105
if err != nil {
106106
t.Fatalf("IssueSession: %v", err)
107107
}
108-
req := httptest.NewRequest(http.MethodGet, "/api/skills", nil)
108+
req := httptest.NewRequest(method, path, nil)
109109
req.AddCookie(cookie)
110110
return req
111111
}
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package setup
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"io"
7+
"net"
8+
"net/http"
9+
"strconv"
10+
"strings"
11+
"testing"
12+
"time"
13+
14+
"github.com/fastclaw-ai/fastclaw/internal/users"
15+
)
16+
17+
var setupRouteTestHTTPClient = &http.Client{Timeout: 2 * time.Second}
18+
19+
func TestListTasksRouteRequiresPlatformAdmin(t *testing.T) {
20+
ctx := context.Background()
21+
s, resolver, adminUser, regularUser := newAuthTestServer(t, ctx)
22+
s.port = freeTCPPort(t)
23+
24+
runCtx, cancel := context.WithCancel(ctx)
25+
defer cancel()
26+
27+
errCh := make(chan error, 1)
28+
go func() {
29+
errCh <- s.Run(runCtx)
30+
}()
31+
baseURL := "http://127.0.0.1:" + strconv.Itoa(s.port)
32+
waitForSetupServer(t, baseURL, errCh)
33+
34+
t.Run("unauthenticated request is rejected", func(t *testing.T) {
35+
resp := tasksListHTTPResponse(t, baseURL, nil)
36+
defer resp.Body.Close()
37+
if resp.StatusCode != http.StatusUnauthorized {
38+
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusUnauthorized)
39+
}
40+
})
41+
42+
t.Run("regular user is rejected", func(t *testing.T) {
43+
cookie, err := resolver.IssueSession(ctx, regularUser.ID)
44+
if err != nil {
45+
t.Fatalf("IssueSession: %v", err)
46+
}
47+
resp := tasksListHTTPResponse(t, baseURL, cookie)
48+
defer resp.Body.Close()
49+
if resp.StatusCode != http.StatusForbidden {
50+
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusForbidden)
51+
}
52+
})
53+
54+
t.Run("super admin is allowed", func(t *testing.T) {
55+
cookie, err := resolver.IssueSession(ctx, adminUser.ID)
56+
if err != nil {
57+
t.Fatalf("IssueSession: %v", err)
58+
}
59+
resp := tasksListHTTPResponse(t, baseURL, cookie)
60+
defer resp.Body.Close()
61+
if resp.StatusCode != http.StatusOK {
62+
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusOK)
63+
}
64+
body, err := io.ReadAll(resp.Body)
65+
if err != nil {
66+
t.Fatalf("ReadAll: %v", err)
67+
}
68+
if got := strings.TrimSpace(string(body)); got != "[]" {
69+
t.Fatalf("body = %q, want []", got)
70+
}
71+
})
72+
73+
t.Run("admin api key is allowed", func(t *testing.T) {
74+
if s.apikeys == nil {
75+
t.Fatal("apikeys not configured")
76+
}
77+
_, token, err := s.apikeys.Create(ctx, adminUser.ID, "tasks-admin", users.APIKeyTypeAdmin, nil)
78+
if err != nil {
79+
t.Fatalf("Create admin apikey: %v", err)
80+
}
81+
resp := tasksListBearerHTTPResponse(t, baseURL, token)
82+
defer resp.Body.Close()
83+
if resp.StatusCode != http.StatusOK {
84+
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusOK)
85+
}
86+
})
87+
88+
cancel()
89+
select {
90+
case err := <-errCh:
91+
if err != nil {
92+
t.Fatalf("Run: %v", err)
93+
}
94+
case <-time.After(2 * time.Second):
95+
t.Fatal("server did not stop after context cancellation")
96+
}
97+
}
98+
99+
func freeTCPPort(t *testing.T) int {
100+
t.Helper()
101+
102+
ln, err := net.Listen("tcp", "127.0.0.1:0")
103+
if err != nil {
104+
t.Fatalf("Listen: %v", err)
105+
}
106+
defer ln.Close()
107+
return ln.Addr().(*net.TCPAddr).Port
108+
}
109+
110+
func waitForSetupServer(t *testing.T, baseURL string, errCh <-chan error) {
111+
t.Helper()
112+
113+
deadline := time.Now().Add(2 * time.Second)
114+
for time.Now().Before(deadline) {
115+
select {
116+
case err := <-errCh:
117+
t.Fatalf("Run exited before server was ready: %v", err)
118+
default:
119+
}
120+
resp, err := setupRouteTestHTTPClient.Get(baseURL + "/healthz")
121+
if err == nil {
122+
_ = resp.Body.Close()
123+
if resp.StatusCode == http.StatusOK {
124+
return
125+
}
126+
}
127+
time.Sleep(10 * time.Millisecond)
128+
}
129+
t.Fatalf("server did not become ready at %s", baseURL)
130+
}
131+
132+
func tasksListHTTPResponse(t *testing.T, baseURL string, cookie *http.Cookie) *http.Response {
133+
t.Helper()
134+
135+
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/tasks", baseURL), nil)
136+
if err != nil {
137+
t.Fatalf("NewRequest: %v", err)
138+
}
139+
if cookie != nil {
140+
req.AddCookie(cookie)
141+
}
142+
resp, err := setupRouteTestHTTPClient.Do(req)
143+
if err != nil {
144+
t.Fatalf("Do: %v", err)
145+
}
146+
return resp
147+
}
148+
149+
func tasksListBearerHTTPResponse(t *testing.T, baseURL, token string) *http.Response {
150+
t.Helper()
151+
152+
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/tasks", baseURL), nil)
153+
if err != nil {
154+
t.Fatalf("NewRequest: %v", err)
155+
}
156+
req.Header.Set("Authorization", "Bearer "+token)
157+
resp, err := setupRouteTestHTTPClient.Do(req)
158+
if err != nil {
159+
t.Fatalf("Do: %v", err)
160+
}
161+
return resp
162+
}

internal/setup/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func (s *Server) Run(ctx context.Context) error {
351351
mux.HandleFunc("PUT /api/agents/{id}/cron/{jobId}", auth(s.handleToggleAgentCronJob))
352352

353353
// Tasks
354-
mux.HandleFunc("GET /api/tasks", auth(s.handleListTasks))
354+
mux.HandleFunc("GET /api/tasks", admin(s.handleListTasks))
355355

356356
// Apikeys (per-user, with agent multi-select).
357357
mux.HandleFunc("GET /api/apikeys", auth(s.handleListAPIKeys))

0 commit comments

Comments
 (0)