Skip to content

Commit c782485

Browse files
yottahmdyottahmd
committed
refactor: apply minor improvements
1 parent 7bbf1ce commit c782485

File tree

12 files changed

+78
-52
lines changed

12 files changed

+78
-52
lines changed

‎internal/agent/bash.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"strings"
1212
"time"
1313

14-
"github.com/dagu-org/dagu/internal/auth"
1514
"github.com/dagu-org/dagu/internal/llm"
1615
"github.com/google/uuid"
1716
)
@@ -142,7 +141,7 @@ func bashRun(toolCtx ToolContext, input json.RawMessage) ToolOut {
142141
if args.Command == "" {
143142
return toolError("Command is required")
144143
}
145-
if toolCtx.Role != auth.Role("") && !toolCtx.Role.CanExecute() {
144+
if toolCtx.Role.IsSet() && !toolCtx.Role.CanExecute() {
146145
return toolError("Permission denied: bash requires execute permission")
147146
}
148147

‎internal/agent/navigate.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"strings"
77

8-
"github.com/dagu-org/dagu/internal/auth"
98
"github.com/dagu-org/dagu/internal/llm"
109
)
1110

@@ -63,7 +62,7 @@ func navigateRun(ctx ToolContext, input json.RawMessage) ToolOut {
6362
if args.Path == "" {
6463
return toolError("Path is required")
6564
}
66-
if isAdminOnlyPath(args.Path) && ctx.Role != auth.Role("") && !ctx.Role.IsAdmin() {
65+
if isAdminOnlyPath(args.Path) && ctx.Role.IsSet() && !ctx.Role.IsAdmin() {
6766
return toolError("Permission denied: navigation to %s requires admin role", args.Path)
6867
}
6968

‎internal/agent/patch.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"path/filepath"
1010
"strings"
1111

12-
"github.com/dagu-org/dagu/internal/auth"
1312
"github.com/dagu-org/dagu/internal/core"
1413
"github.com/dagu-org/dagu/internal/core/spec"
1514
"github.com/dagu-org/dagu/internal/llm"
@@ -87,7 +86,7 @@ func NewPatchTool(dagsDir string) *AgentTool {
8786
}
8887

8988
func patchRun(ctx ToolContext, input json.RawMessage, dagsDir string) ToolOut {
90-
if ctx.Role != auth.Role("") && !ctx.Role.CanWrite() {
89+
if ctx.Role.IsSet() && !ctx.Role.CanWrite() {
9190
return toolError("Permission denied: patch requires write permission")
9291
}
9392

‎internal/agent/system_prompt_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ func TestGenerateSystemPrompt(t *testing.T) {
3939

4040
assert.NotEmpty(t, result)
4141
assert.Contains(t, result, "test-dag")
42+
assert.Contains(t, result, "Authenticated role: admin")
4243
})
4344

4445
t.Run("works with empty environment", func(t *testing.T) {
@@ -47,6 +48,7 @@ func TestGenerateSystemPrompt(t *testing.T) {
4748
result := GenerateSystemPrompt(EnvironmentInfo{}, nil, MemoryContent{}, auth.RoleViewer)
4849

4950
assert.NotEmpty(t, result)
51+
assert.Contains(t, result, "Authenticated role: viewer")
5052
})
5153

5254
t.Run("no memory omits memory section", func(t *testing.T) {

‎internal/auth/role.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ const (
2424
RoleOperator Role = "operator"
2525
// RoleViewer can only view DAGs and execution history (read-only).
2626
RoleViewer Role = "viewer"
27+
// RoleNone represents an unset or unauthenticated role.
28+
RoleNone Role = ""
2729
)
2830

2931
// allRoles contains all valid roles for iteration and validation.
@@ -41,6 +43,8 @@ func (r Role) Valid() bool {
4143
switch r {
4244
case RoleAdmin, RoleManager, RoleDeveloper, RoleOperator, RoleViewer:
4345
return true
46+
case RoleNone:
47+
return false
4448
}
4549
return false
4650
}
@@ -50,6 +54,11 @@ func (r Role) String() string {
5054
return string(r)
5155
}
5256

57+
// IsSet returns true if the role has been assigned (is not empty).
58+
func (r Role) IsSet() bool {
59+
return r != RoleNone
60+
}
61+
5362
// CanWrite returns true if the role can create, edit, or delete DAGs.
5463
func (r Role) CanWrite() bool {
5564
return r == RoleAdmin || r == RoleManager || r == RoleDeveloper
@@ -71,7 +80,6 @@ func (r Role) IsAdmin() bool {
7180
}
7281

7382
// ParseRole converts a string to a Role.
74-
// ParseRole converts the input string to a Role and verifies it is one of the known roles.
7583
// If the input is not "admin", "manager", "developer", "operator", or "viewer", it returns an error describing the valid options.
7684
func ParseRole(s string) (Role, error) {
7785
role := Role(s)

‎internal/service/frontend/api/v1/audit_permissions_test.go‎

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package api_test
33
import (
44
"net/http"
55
"testing"
6-
"time"
76

87
"github.com/dagu-org/dagu/api/v1"
98
"github.com/dagu-org/dagu/internal/cmn/config"
@@ -12,54 +11,64 @@ import (
1211

1312
func setupAuditTestServer(t *testing.T) test.Server {
1413
t.Helper()
15-
return test.SetupServer(t, test.WithConfigMutator(func(cfg *config.Config) {
16-
cfg.Server.Auth.Mode = config.AuthModeBuiltin
17-
cfg.Server.Auth.Builtin.Admin.Username = "admin"
18-
cfg.Server.Auth.Builtin.Admin.Password = "adminpass"
19-
cfg.Server.Auth.Builtin.Token.Secret = "jwt-secret-key"
20-
cfg.Server.Auth.Builtin.Token.TTL = 24 * time.Hour
14+
return setupWebhookTestServer(t, func(cfg *config.Config) {
2115
cfg.Server.Audit.Enabled = true
22-
}))
16+
})
2317
}
2418

2519
func TestAudit_RequiresManagerOrAbove(t *testing.T) {
2620
t.Parallel()
2721
server := setupAuditTestServer(t)
2822
adminToken := getWebhookAdminToken(t, server)
2923

30-
server.Client().Post("/api/v1/users", api.CreateUserRequest{
31-
Username: "manager-user",
32-
Password: "manager1",
33-
Role: api.UserRoleManager,
34-
}).WithBearerToken(adminToken).ExpectStatus(http.StatusCreated).Send(t)
24+
// Create users for each role below manager.
25+
for _, u := range []struct {
26+
username string
27+
password string
28+
role api.UserRole
29+
}{
30+
{"manager-user", "manager1", api.UserRoleManager},
31+
{"developer-user", "developer1", api.UserRoleDeveloper},
32+
{"operator-user", "operator1", api.UserRoleOperator},
33+
{"viewer-user", "viewerpass1", api.UserRoleViewer},
34+
} {
35+
server.Client().Post("/api/v1/users", api.CreateUserRequest{
36+
Username: u.username,
37+
Password: u.password,
38+
Role: u.role,
39+
}).WithBearerToken(adminToken).ExpectStatus(http.StatusCreated).Send(t)
40+
}
3541

36-
server.Client().Post("/api/v1/users", api.CreateUserRequest{
37-
Username: "developer-user",
38-
Password: "developer1",
39-
Role: api.UserRoleDeveloper,
40-
}).WithBearerToken(adminToken).ExpectStatus(http.StatusCreated).Send(t)
42+
login := func(username, password string) string {
43+
resp := server.Client().Post("/api/v1/auth/login", api.LoginRequest{
44+
Username: username,
45+
Password: password,
46+
}).ExpectStatus(http.StatusOK).Send(t)
47+
var result api.LoginResponse
48+
resp.Unmarshal(t, &result)
49+
return result.Token
50+
}
4151

42-
managerResp := server.Client().Post("/api/v1/auth/login", api.LoginRequest{
43-
Username: "manager-user",
44-
Password: "manager1",
45-
}).ExpectStatus(http.StatusOK).Send(t)
52+
managerToken := login("manager-user", "manager1")
53+
developerToken := login("developer-user", "developer1")
54+
operatorToken := login("operator-user", "operator1")
55+
viewerToken := login("viewer-user", "viewerpass1")
4656

47-
var managerLogin api.LoginResponse
48-
managerResp.Unmarshal(t, &managerLogin)
49-
50-
developerResp := server.Client().Post("/api/v1/auth/login", api.LoginRequest{
51-
Username: "developer-user",
52-
Password: "developer1",
53-
}).ExpectStatus(http.StatusOK).Send(t)
57+
// Manager can access audit endpoint.
58+
server.Client().Get("/api/v1/audit").
59+
WithBearerToken(managerToken).
60+
ExpectStatus(http.StatusOK).Send(t)
5461

55-
var developerLogin api.LoginResponse
56-
developerResp.Unmarshal(t, &developerLogin)
62+
// Developer, operator, and viewer are forbidden.
63+
server.Client().Get("/api/v1/audit").
64+
WithBearerToken(developerToken).
65+
ExpectStatus(http.StatusForbidden).Send(t)
5766

5867
server.Client().Get("/api/v1/audit").
59-
WithBearerToken(managerLogin.Token).
60-
ExpectStatus(http.StatusOK).Send(t)
68+
WithBearerToken(operatorToken).
69+
ExpectStatus(http.StatusForbidden).Send(t)
6170

6271
server.Client().Get("/api/v1/audit").
63-
WithBearerToken(developerLogin.Token).
72+
WithBearerToken(viewerToken).
6473
ExpectStatus(http.StatusForbidden).Send(t)
6574
}

‎internal/service/frontend/api/v1/webhooks.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const (
2727
)
2828

2929
// ListWebhooks returns all webhooks across all DAGs.
30-
// Requires admin role.
30+
// Requires developer role or above.
3131
func (a *API) ListWebhooks(ctx context.Context, _ api.ListWebhooksRequestObject) (api.ListWebhooksResponseObject, error) {
3232
if err := a.requireWebhookManagement(ctx); err != nil {
3333
return nil, err
@@ -52,7 +52,7 @@ func (a *API) ListWebhooks(ctx context.Context, _ api.ListWebhooksRequestObject)
5252
}
5353

5454
// GetDAGWebhook returns the webhook configuration for a specific DAG.
55-
// Requires admin role.
55+
// Requires developer role or above.
5656
func (a *API) GetDAGWebhook(ctx context.Context, request api.GetDAGWebhookRequestObject) (api.GetDAGWebhookResponseObject, error) {
5757
if err := a.requireWebhookManagement(ctx); err != nil {
5858
return nil, err
@@ -79,7 +79,7 @@ func (a *API) GetDAGWebhook(ctx context.Context, request api.GetDAGWebhookReques
7979
}
8080

8181
// CreateDAGWebhook creates a new webhook for a DAG.
82-
// Requires admin role.
82+
// Requires developer role or above.
8383
func (a *API) CreateDAGWebhook(ctx context.Context, request api.CreateDAGWebhookRequestObject) (api.CreateDAGWebhookResponseObject, error) {
8484
if err := a.requireWebhookManagement(ctx); err != nil {
8585
return nil, err
@@ -130,7 +130,7 @@ func (a *API) CreateDAGWebhook(ctx context.Context, request api.CreateDAGWebhook
130130
}
131131

132132
// DeleteDAGWebhook removes the webhook for a DAG.
133-
// Requires admin role.
133+
// Requires developer role or above.
134134
func (a *API) DeleteDAGWebhook(ctx context.Context, request api.DeleteDAGWebhookRequestObject) (api.DeleteDAGWebhookResponseObject, error) {
135135
if err := a.requireWebhookManagement(ctx); err != nil {
136136
return nil, err
@@ -167,7 +167,7 @@ func (a *API) DeleteDAGWebhook(ctx context.Context, request api.DeleteDAGWebhook
167167
}
168168

169169
// RegenerateDAGWebhookToken generates a new token for an existing webhook.
170-
// Requires admin role.
170+
// Requires developer role or above.
171171
func (a *API) RegenerateDAGWebhookToken(ctx context.Context, request api.RegenerateDAGWebhookTokenRequestObject) (api.RegenerateDAGWebhookTokenResponseObject, error) {
172172
if err := a.requireWebhookManagement(ctx); err != nil {
173173
return nil, err
@@ -203,7 +203,7 @@ func (a *API) RegenerateDAGWebhookToken(ctx context.Context, request api.Regener
203203
}
204204

205205
// ToggleDAGWebhook enables or disables a webhook.
206-
// Requires admin role.
206+
// Requires developer role or above.
207207
func (a *API) ToggleDAGWebhook(ctx context.Context, request api.ToggleDAGWebhookRequestObject) (api.ToggleDAGWebhookResponseObject, error) {
208208
if err := a.requireWebhookManagement(ctx); err != nil {
209209
return nil, err

‎internal/service/frontend/api/v1/webhooks_test.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,17 @@ func TestExtractWebhookToken(t *testing.T) {
6363
}
6464

6565
// setupWebhookTestServer creates a test server with builtin auth enabled
66-
func setupWebhookTestServer(t *testing.T) test.Server {
66+
func setupWebhookTestServer(t *testing.T, extraMutators ...func(*config.Config)) test.Server {
6767
t.Helper()
6868
return test.SetupServer(t, test.WithConfigMutator(func(cfg *config.Config) {
6969
cfg.Server.Auth.Mode = config.AuthModeBuiltin
7070
cfg.Server.Auth.Builtin.Admin.Username = "admin"
7171
cfg.Server.Auth.Builtin.Admin.Password = "adminpass"
7272
cfg.Server.Auth.Builtin.Token.Secret = "jwt-secret-key"
7373
cfg.Server.Auth.Builtin.Token.TTL = 24 * time.Hour
74+
for _, m := range extraMutators {
75+
m(cfg)
76+
}
7477
}))
7578
}
7679

@@ -204,6 +207,13 @@ func TestWebhooks_RequiresDeveloperOrAbove(t *testing.T) {
204207
server.Client().Get("/api/v1/webhooks").
205208
WithBearerToken(developerLogin.Token).
206209
ExpectStatus(http.StatusOK).Send(t)
210+
211+
// Developer can also create webhooks.
212+
dagName := "webhook_dev_access_test"
213+
createTestDAG(t, server, adminToken, dagName)
214+
server.Client().Post("/api/v1/dags/"+dagName+"/webhook", nil).
215+
WithBearerToken(developerLogin.Token).
216+
ExpectStatus(http.StatusCreated).Send(t)
207217
}
208218

209219
// TestWebhooks_CRUD tests the full CRUD lifecycle of webhooks

‎internal/service/frontend/api/v1/workers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import (
1313

1414
// GetWorkers implements the getWorkers operation
1515
func (a *API) GetWorkers(ctx context.Context, _ api.GetWorkersRequestObject) (api.GetWorkersResponseObject, error) {
16-
logger.Info(ctx, "GetWorkers called")
1716
if err := a.requireDeveloperOrAbove(ctx); err != nil {
1817
return nil, err
1918
}
19+
logger.Info(ctx, "GetWorkers called")
2020

2121
errors := []string{}
2222
workers := []api.Worker{}

‎ui/src/features/dashboard/components/MiniResourceChart.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ function MiniResourceChart({
8989
}}
9090
itemStyle={{ color: 'var(--foreground)' }}
9191
labelStyle={{ color: 'var(--muted-foreground)' }}
92-
formatter={(value: number) => [`${value.toFixed(1)}${unit}`, title]}
92+
formatter={(value: number | string) => [`${Number(value).toFixed(1)}${unit}`, title]}
9393
/>
9494
<Area
9595
type="monotone"

0 commit comments

Comments
 (0)