Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit416337a

Browse files
committed
more updates and test refactors
Signed-off-by: Callum Styan <callumstyan@gmail.com>
1 parentb5356b7 commit416337a

File tree

6 files changed

+234
-17
lines changed

6 files changed

+234
-17
lines changed

‎coderd/agentapi/cached_workspace.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package agentapi
22

33
import (
4-
"sync"
54
"fmt"
5+
"sync"
66

77
"github.com/coder/coder/v2/coderd/database"
88
)

‎coderd/agentapi/connectionlog.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ package agentapi
33
import (
44
"context"
55
"database/sql"
6-
"sync/atomic"
76
"fmt"
7+
"sync/atomic"
88

99
"github.com/google/uuid"
1010
"golang.org/x/xerrors"
@@ -13,9 +13,9 @@ import (
1313
"cdr.dev/slog"
1414
agentproto"github.com/coder/coder/v2/agent/proto"
1515
"github.com/coder/coder/v2/coderd/connectionlog"
16-
"github.com/coder/coder/v2/coderd/database/dbauthz"
1716
"github.com/coder/coder/v2/coderd/database"
1817
"github.com/coder/coder/v2/coderd/database/db2sdk"
18+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1919
)
2020

2121
typeConnLogAPIstruct {
@@ -81,7 +81,6 @@ func (a *ConnLogAPI) ReportConnection(ctx context.Context, req *agentproto.Repor
8181
}
8282
ws=database.WorkspaceIdentityFromWorkspace(workspace)
8383
}
84-
8584

8685
// Some older clients may incorrectly report "localhost" as the IP address.
8786
// Related to https://github.com/coder/coder/issues/20194

‎coderd/agentapi/connectionlog_test.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,16 +205,15 @@ func TestWorkspaceCached_SkipsDBCall(t *testing.T) {
205205
ID:uuid.New(),
206206
}
207207
)
208-
id:=uuid.New()
209-
action:=agentproto.Connection_CONNECT.Enum()
210-
typ:=agentproto.Connection_SSH.Enum()
211-
time:=dbtime.Now()
212-
ip:="127.0.0.1"
213-
status:=200
208+
id:=uuid.New()
209+
action:=agentproto.Connection_CONNECT.Enum()
210+
typ:=agentproto.Connection_SSH.Enum()
211+
time:=dbtime.Now()
212+
ip:="127.0.0.1"
213+
status:=200
214214
reason:=""
215215
cachedWorkspace:=&agentapi.CachedWorkspaceFields{}
216216

217-
218217
cachedWorkspace.UpdateValues(workspace)
219218

220219
connLogger:=connectionlog.NewFake()

‎coderd/agentapi/metadata_test.go‎

Lines changed: 129 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,48 @@ func TestBatchUpdateMetadata(t *testing.T) {
359359
OrganizationID:orgID,
360360
})
361361

362-
// Create context with system actor so authorization passes
363-
ctx:=dbauthz.AsSystemRestricted(context.Background())
362+
// Create roles with workspace permissions
363+
userRoles:=rbac.Roles([]rbac.Role{
364+
{
365+
Identifier:rbac.RoleMember(),
366+
User: []rbac.Permission{
367+
{
368+
Negate:false,
369+
ResourceType:rbac.ResourceWorkspace.Type,
370+
Action:policy.WildcardSymbol,
371+
},
372+
},
373+
ByOrgID:map[string]rbac.OrgPermissions{
374+
orgID.String(): {
375+
Member: []rbac.Permission{
376+
{
377+
Negate:false,
378+
ResourceType:rbac.ResourceWorkspace.Type,
379+
Action:policy.WildcardSymbol,
380+
},
381+
},
382+
},
383+
},
384+
},
385+
})
386+
387+
agentScope:=rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
388+
WorkspaceID:workspaceID,
389+
OwnerID:ownerID,
390+
TemplateID:templateID,
391+
VersionID:uuid.New(),
392+
})
393+
394+
ctx:=dbauthz.As(context.Background(), rbac.Subject{
395+
Type:rbac.SubjectTypeUser,
396+
FriendlyName:"testuser",
397+
Email:"testuser@example.com",
398+
ID:ownerID.String(),
399+
Roles:userRoles,
400+
Groups: []string{orgID.String()},
401+
Scope:agentScope,
402+
}.WithCachedASTValue())
403+
364404
resp,err:=api.BatchUpdateMetadata(ctx,req)
365405
require.NoError(t,err)
366406
require.NotNil(t,resp)
@@ -377,6 +417,7 @@ func TestBatchUpdateMetadata(t *testing.T) {
377417
pub=&fakePublisher{}
378418
now=dbtime.Now()
379419
workspaceID=uuid.MustParse("12345678-1234-1234-1234-123456789012")
420+
templateID=uuid.MustParse("aaaabbbb-cccc-dddd-eeee-ffffffff0000")
380421
ownerID=uuid.MustParse("87654321-4321-4321-4321-210987654321")
381422
orgID=uuid.MustParse("11111111-1111-1111-1111-111111111111")
382423
agentID=uuid.MustParse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb")
@@ -446,12 +487,53 @@ func TestBatchUpdateMetadata(t *testing.T) {
446487
OrganizationID:uuid.Nil,// Invalid: fails dbauthz fast path validation
447488
})
448489

449-
// Create context with system actor so authorization passes
450-
ctx:=dbauthz.AsSystemRestricted(context.Background())
490+
// Create roles with workspace permissions
491+
userRoles:=rbac.Roles([]rbac.Role{
492+
{
493+
Identifier:rbac.RoleMember(),
494+
User: []rbac.Permission{
495+
{
496+
Negate:false,
497+
ResourceType:rbac.ResourceWorkspace.Type,
498+
Action:policy.WildcardSymbol,
499+
},
500+
},
501+
ByOrgID:map[string]rbac.OrgPermissions{
502+
orgID.String(): {
503+
Member: []rbac.Permission{
504+
{
505+
Negate:false,
506+
ResourceType:rbac.ResourceWorkspace.Type,
507+
Action:policy.WildcardSymbol,
508+
},
509+
},
510+
},
511+
},
512+
},
513+
})
514+
515+
agentScope:=rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
516+
WorkspaceID:workspaceID,
517+
OwnerID:ownerID,
518+
TemplateID:templateID,
519+
VersionID:uuid.New(),
520+
})
521+
522+
ctx:=dbauthz.As(context.Background(), rbac.Subject{
523+
Type:rbac.SubjectTypeUser,
524+
FriendlyName:"testuser",
525+
Email:"testuser@example.com",
526+
ID:ownerID.String(),
527+
Roles:userRoles,
528+
Groups: []string{orgID.String()},
529+
Scope:agentScope,
530+
}.WithCachedASTValue())
531+
451532
resp,err:=api.BatchUpdateMetadata(ctx,req)
452533
require.NoError(t,err)
453534
require.NotNil(t,resp)
454535
})
536+
455537
// Test RBAC slow path - no RBAC object in context
456538
// This test verifies that when no RBAC object is present in context, the dbauthz layer
457539
// falls back to the slow path and calls GetWorkspaceByAgentID.
@@ -464,6 +546,7 @@ func TestBatchUpdateMetadata(t *testing.T) {
464546
pub=&fakePublisher{}
465547
now=dbtime.Now()
466548
workspaceID=uuid.MustParse("12345678-1234-1234-1234-123456789012")
549+
templateID=uuid.MustParse("aaaabbbb-cccc-dddd-eeee-ffffffff0000")
467550
ownerID=uuid.MustParse("87654321-4321-4321-4321-210987654321")
468551
orgID=uuid.MustParse("11111111-1111-1111-1111-111111111111")
469552
agentID=uuid.MustParse("dddddddd-dddd-dddd-dddd-dddddddddddd")
@@ -524,8 +607,48 @@ func TestBatchUpdateMetadata(t *testing.T) {
524607
},
525608
}
526609

527-
// Create context with system actor so authorization passes
528-
ctx:=dbauthz.AsSystemRestricted(context.Background())
610+
// Create roles with workspace permissions
611+
userRoles:=rbac.Roles([]rbac.Role{
612+
{
613+
Identifier:rbac.RoleMember(),
614+
User: []rbac.Permission{
615+
{
616+
Negate:false,
617+
ResourceType:rbac.ResourceWorkspace.Type,
618+
Action:policy.WildcardSymbol,
619+
},
620+
},
621+
ByOrgID:map[string]rbac.OrgPermissions{
622+
orgID.String(): {
623+
Member: []rbac.Permission{
624+
{
625+
Negate:false,
626+
ResourceType:rbac.ResourceWorkspace.Type,
627+
Action:policy.WildcardSymbol,
628+
},
629+
},
630+
},
631+
},
632+
},
633+
})
634+
635+
agentScope:=rbac.WorkspaceAgentScope(rbac.WorkspaceAgentScopeParams{
636+
WorkspaceID:workspaceID,
637+
OwnerID:ownerID,
638+
TemplateID:templateID,
639+
VersionID:uuid.New(),
640+
})
641+
642+
ctx:=dbauthz.As(context.Background(), rbac.Subject{
643+
Type:rbac.SubjectTypeUser,
644+
FriendlyName:"testuser",
645+
Email:"testuser@example.com",
646+
ID:ownerID.String(),
647+
Roles:userRoles,
648+
Groups: []string{orgID.String()},
649+
Scope:agentScope,
650+
}.WithCachedASTValue())
651+
529652
resp,err:=api.BatchUpdateMetadata(ctx,req)
530653
require.NoError(t,err)
531654
require.NotNil(t,resp)

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"database/sql"
66
"encoding/json"
77
"errors"
8+
"fmt"
89
"slices"
910
"strings"
1011
"sync/atomic"
@@ -3523,6 +3524,23 @@ func (q *querier) GetWorkspaceAgentAndLatestBuildByAuthToken(ctx context.Context
35233524
}
35243525

35253526
func (q*querier)GetWorkspaceAgentByID(ctx context.Context,id uuid.UUID) (database.WorkspaceAgent,error) {
3527+
// Fast path: Check if we have a workspace RBAC object in context.
3528+
// In the agent API this is set at agent connection time to avoid the expensive
3529+
// GetWorkspaceByAgentID query for every agent operation.
3530+
// NOTE: The cached RBAC object is refreshed every 5 minutes in agentapi/api.go.
3531+
ifrbacObj,ok:=WorkspaceRBACFromContext(ctx);ok {
3532+
// Errors here will result in falling back to GetWorkspaceByAgentID,
3533+
// in case the cached data is stale.
3534+
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbacObj);err==nil {
3535+
fmt.Println("skipping getworkspacebyagentID call")
3536+
returnq.db.GetWorkspaceAgentByID(ctx,id)
3537+
}
3538+
q.log.Debug(ctx,"fast path authorization failed for GetWorkspaceAgentByID, using slow path",
3539+
slog.F("agent_id",id))
3540+
}
3541+
3542+
// Slow path: Fallback to fetching the workspace for authorization
3543+
fmt.Println("fallback")
35263544
if_,err:=q.GetWorkspaceByAgentID(ctx,id);err!=nil {
35273545
return database.WorkspaceAgent{},err
35283546
}

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4707,3 +4707,81 @@ func (s *MethodTestSuite) TestTelemetry() {
47074707
check.Args(database.CalculateAIBridgeInterceptionsTelemetrySummaryParams{}).Asserts(rbac.ResourceAibridgeInterception,policy.ActionRead)
47084708
}))
47094709
}
4710+
4711+
funcTestGetWorkspaceAgentByID_FastPath(t*testing.T) {
4712+
t.Parallel()
4713+
4714+
agentID:=uuid.New()
4715+
ownerID:=uuid.New()
4716+
wsID:=uuid.New()
4717+
orgID:=uuid.New()
4718+
4719+
agent:= database.WorkspaceAgent{
4720+
ID:agentID,
4721+
Name:"test-agent",
4722+
}
4723+
4724+
workspace:= database.Workspace{
4725+
ID:wsID,
4726+
OwnerID:ownerID,
4727+
OrganizationID:orgID,
4728+
}
4729+
4730+
wsIdentity:= database.WorkspaceIdentity{
4731+
ID:wsID,
4732+
OwnerID:ownerID,
4733+
OrganizationID:orgID,
4734+
}
4735+
4736+
actor:= rbac.Subject{
4737+
ID:ownerID.String(),
4738+
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
4739+
Groups: []string{orgID.String()},
4740+
Scope:rbac.ScopeAll,
4741+
}
4742+
4743+
authorizer:=&coderdtest.RecordingAuthorizer{
4744+
Wrapped: (&coderdtest.FakeAuthorizer{}).AlwaysReturn(nil),
4745+
}
4746+
4747+
t.Run("WithWorkspaceRBAC",func(t*testing.T) {
4748+
t.Parallel()
4749+
4750+
ctx:=dbauthz.As(context.Background(),actor)
4751+
ctrl:=gomock.NewController(t)
4752+
mockDB:=dbmock.NewMockStore(ctrl)
4753+
4754+
rbacObj:=wsIdentity.RBACObject()
4755+
ctx,err:=dbauthz.WithWorkspaceRBAC(ctx,rbacObj)
4756+
require.NoError(t,err)
4757+
4758+
mockDB.EXPECT().Wrappers().Return([]string{})
4759+
// GetWorkspaceByAgentID should NOT be called
4760+
mockDB.EXPECT().GetWorkspaceAgentByID(gomock.Any(),agentID).Return(agent,nil)
4761+
4762+
q:=dbauthz.New(mockDB,authorizer,slogtest.Make(t,nil),coderdtest.AccessControlStorePointer())
4763+
4764+
result,err:=q.GetWorkspaceAgentByID(ctx,agentID)
4765+
require.NoError(t,err)
4766+
require.Equal(t,agent,result)
4767+
})
4768+
4769+
t.Run("WithoutWorkspaceRBAC",func(t*testing.T) {
4770+
t.Parallel()
4771+
4772+
ctx:=dbauthz.As(context.Background(),actor)
4773+
ctrl:=gomock.NewController(t)
4774+
mockDB:=dbmock.NewMockStore(ctrl)
4775+
4776+
mockDB.EXPECT().Wrappers().Return([]string{})
4777+
// GetWorkspaceByAgentID SHOULD be called
4778+
mockDB.EXPECT().GetWorkspaceByAgentID(gomock.Any(),agentID).Return(workspace,nil)
4779+
mockDB.EXPECT().GetWorkspaceAgentByID(gomock.Any(),agentID).Return(agent,nil)
4780+
4781+
q:=dbauthz.New(mockDB,authorizer,slogtest.Make(t,nil),coderdtest.AccessControlStorePointer())
4782+
4783+
result,err:=q.GetWorkspaceAgentByID(ctx,agentID)
4784+
require.NoError(t,err)
4785+
require.Equal(t,agent,result)
4786+
})
4787+
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp