Skip to content

Commit 21fa63c

Browse files
ethanndicksonethanndickson
committed
auth-filter query
1 parent 14796a9 commit 21fa63c

File tree

9 files changed

+170
-54
lines changed

9 files changed

+170
-54
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2720,7 +2720,11 @@ func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesP
27202720
}
27212721

27222722
func (q *querier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
2723-
return q.db.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
2723+
prep, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceWorkspace.Type)
2724+
if err != nil {
2725+
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
2726+
}
2727+
return q.db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, prep)
27242728
}
27252729

27262730
func (q *querier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
@@ -4168,6 +4172,10 @@ func (q *querier) GetAuthorizedWorkspaces(ctx context.Context, arg database.GetW
41684172
return q.GetWorkspaces(ctx, arg)
41694173
}
41704174

4175+
func (q *querier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, _ rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
4176+
return q.GetWorkspacesAndAgentsByOwnerID(ctx, ownerID)
4177+
}
4178+
41714179
// GetAuthorizedUsers is not required for dbauthz since GetUsers is already
41724180
// authenticated.
41734181
func (q *querier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, _ rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,6 +1478,14 @@ func (s *MethodTestSuite) TestWorkspace() {
14781478
_ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
14791479
check.Args(ws.OwnerID).Asserts()
14801480
}))
1481+
s.Run("GetAuthorizedWorkspacesAndAgentsByOwnerID", s.Subtest(func(db database.Store, check *expects) {
1482+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1483+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1484+
_ = dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild})
1485+
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1486+
_ = dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
1487+
check.Args(ws.OwnerID, emptyPreparedAuthorized{}).Asserts()
1488+
}))
14811489
s.Run("GetLatestWorkspaceBuildByWorkspaceID", s.Subtest(func(db database.Store, check *expects) {
14821490
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
14831491
b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})

‎coderd/database/dbmem/dbmem.go

Lines changed: 63 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6795,57 +6795,8 @@ func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspa
67956795
}
67966796

67976797
func (q *FakeQuerier) GetWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
6798-
q.mutex.RLock()
6799-
defer q.mutex.RUnlock()
6800-
6801-
workspaces := make([]database.Workspace, 0)
6802-
for _, workspace := range q.workspaces {
6803-
if workspace.OwnerID == ownerID && !workspace.Deleted {
6804-
workspaces = append(workspaces, workspace)
6805-
}
6806-
}
6807-
6808-
out := make([]database.GetWorkspacesAndAgentsByOwnerIDRow, 0, len(workspaces))
6809-
for _, w := range workspaces {
6810-
// these always exist
6811-
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
6812-
if err != nil {
6813-
return nil, xerrors.Errorf("get latest build: %w", err)
6814-
}
6815-
6816-
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
6817-
if err != nil {
6818-
return nil, xerrors.Errorf("get provisioner job: %w", err)
6819-
}
6820-
6821-
outAgents := make([]database.AgentIDNamePair, 0)
6822-
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
6823-
if err != nil {
6824-
return nil, xerrors.Errorf("get workspace resources: %w", err)
6825-
}
6826-
if len(resources) > 0 {
6827-
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
6828-
if err != nil {
6829-
return nil, xerrors.Errorf("get workspace agents: %w", err)
6830-
}
6831-
for _, a := range agents {
6832-
outAgents = append(outAgents, database.AgentIDNamePair{
6833-
ID: a.ID,
6834-
Name: a.Name,
6835-
})
6836-
}
6837-
}
6838-
6839-
out = append(out, database.GetWorkspacesAndAgentsByOwnerIDRow{
6840-
ID: w.ID,
6841-
Name: w.Name,
6842-
JobStatus: job.JobStatus,
6843-
Transition: build.Transition,
6844-
Agents: outAgents,
6845-
})
6846-
}
6847-
6848-
return out, nil
6798+
// No auth filter.
6799+
return q.GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx, ownerID, nil)
68496800
}
68506801

68516802
func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]database.Workspace, error) {
@@ -11227,6 +11178,67 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
1122711178
return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount), arg.WithSummary), nil
1122811179
}
1122911180

11181+
func (q *FakeQuerier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]database.GetWorkspacesAndAgentsByOwnerIDRow, error) {
11182+
q.mutex.RLock()
11183+
defer q.mutex.RUnlock()
11184+
11185+
if prepared != nil {
11186+
// Call this to match the same function calls as the SQL implementation.
11187+
_, err := prepared.CompileToSQL(ctx, rbac.ConfigWithoutACL())
11188+
if err != nil {
11189+
return nil, err
11190+
}
11191+
}
11192+
workspaces := make([]database.Workspace, 0)
11193+
for _, workspace := range q.workspaces {
11194+
if workspace.OwnerID == ownerID && !workspace.Deleted {
11195+
workspaces = append(workspaces, workspace)
11196+
}
11197+
}
11198+
11199+
out := make([]database.GetWorkspacesAndAgentsByOwnerIDRow, 0, len(workspaces))
11200+
for _, w := range workspaces {
11201+
// these always exist
11202+
build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID)
11203+
if err != nil {
11204+
return nil, xerrors.Errorf("get latest build: %w", err)
11205+
}
11206+
11207+
job, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID)
11208+
if err != nil {
11209+
return nil, xerrors.Errorf("get provisioner job: %w", err)
11210+
}
11211+
11212+
outAgents := make([]database.AgentIDNamePair, 0)
11213+
resources, err := q.getWorkspaceResourcesByJobIDNoLock(ctx, job.ID)
11214+
if err != nil {
11215+
return nil, xerrors.Errorf("get workspace resources: %w", err)
11216+
}
11217+
if len(resources) > 0 {
11218+
agents, err := q.getWorkspaceAgentsByResourceIDsNoLock(ctx, []uuid.UUID{resources[0].ID})
11219+
if err != nil {
11220+
return nil, xerrors.Errorf("get workspace agents: %w", err)
11221+
}
11222+
for _, a := range agents {
11223+
outAgents = append(outAgents, database.AgentIDNamePair{
11224+
ID: a.ID,
11225+
Name: a.Name,
11226+
})
11227+
}
11228+
}
11229+
11230+
out = append(out, database.GetWorkspacesAndAgentsByOwnerIDRow{
11231+
ID: w.ID,
11232+
Name: w.Name,
11233+
JobStatus: job.JobStatus,
11234+
Transition: build.Transition,
11235+
Agents: outAgents,
11236+
})
11237+
}
11238+
11239+
return out, nil
11240+
}
11241+
1123011242
func (q *FakeQuerier) GetAuthorizedUsers(ctx context.Context, arg database.GetUsersParams, prepared rbac.PreparedAuthorized) ([]database.GetUsersRow, error) {
1123111243
if err := validateDatabaseType(arg); err != nil {
1123211244
return nil, err

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/modelqueries.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ func (q *sqlQuerier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([
221221

222222
type workspaceQuerier interface {
223223
GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, prepared rbac.PreparedAuthorized) ([]GetWorkspacesRow, error)
224+
GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsByOwnerIDRow, error)
224225
}
225226

226227
// GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access.
@@ -312,6 +313,49 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa
312313
return items, nil
313314
}
314315

316+
func (q *sqlQuerier) GetAuthorizedWorkspacesAndAgentsByOwnerID(ctx context.Context, ownerID uuid.UUID, prepared rbac.PreparedAuthorized) ([]GetWorkspacesAndAgentsByOwnerIDRow, error) {
317+
authorizedFilter, err := prepared.CompileToSQL(ctx, rbac.ConfigWorkspaces())
318+
if err != nil {
319+
return nil, xerrors.Errorf("compile authorized filter: %w", err)
320+
}
321+
322+
// In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the
323+
// authorizedFilter between the end of the where clause and those statements.
324+
filtered, err := insertAuthorizedFilter(getWorkspacesAndAgentsByOwnerID, fmt.Sprintf(" AND %s", authorizedFilter))
325+
if err != nil {
326+
return nil, xerrors.Errorf("insert authorized filter: %w", err)
327+
}
328+
329+
// The name comment is for metric tracking
330+
query := fmt.Sprintf("-- name: GetAuthorizedWorkspacesAndAgentsByOwnerID :many\n%s", filtered)
331+
rows, err := q.db.QueryContext(ctx, query, ownerID)
332+
if err != nil {
333+
return nil, err
334+
}
335+
defer rows.Close()
336+
var items []GetWorkspacesAndAgentsByOwnerIDRow
337+
for rows.Next() {
338+
var i GetWorkspacesAndAgentsByOwnerIDRow
339+
if err := rows.Scan(
340+
&i.ID,
341+
&i.Name,
342+
&i.JobStatus,
343+
&i.Transition,
344+
pq.Array(&i.Agents),
345+
); err != nil {
346+
return nil, err
347+
}
348+
items = append(items, i)
349+
}
350+
if err := rows.Close(); err != nil {
351+
return nil, err
352+
}
353+
if err := rows.Err(); err != nil {
354+
return nil, err
355+
}
356+
return items, nil
357+
}
358+
315359
type userQuerier interface {
316360
GetAuthorizedUsers(ctx context.Context, arg GetUsersParams, prepared rbac.PreparedAuthorized) ([]GetUsersRow, error)
317361
}

‎coderd/database/querier_test.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import (
2424
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2525
"github.com/coder/coder/v2/coderd/database/dbtime"
2626
"github.com/coder/coder/v2/coderd/database/migrations"
27+
"github.com/coder/coder/v2/coderd/httpmw"
2728
"github.com/coder/coder/v2/coderd/rbac"
29+
"github.com/coder/coder/v2/coderd/rbac/policy"
2830
"github.com/coder/coder/v2/testutil"
2931
)
3032

@@ -612,7 +614,7 @@ func TestGetWorkspaceAgentUsageStatsAndLabels(t *testing.T) {
612614
})
613615
}
614616

615-
func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
617+
func TestGetAuthorizedWorkspacesAndAgentsByOwnerID(t *testing.T) {
616618
t.Parallel()
617619
if testing.Short() {
618620
t.SkipNow()
@@ -628,6 +630,7 @@ func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
628630
owner := dbgen.User(t, db, database.User{
629631
RBACRoles: []string{rbac.RoleOwner().String()},
630632
})
633+
user := dbgen.User(t, db, database.User{})
631634
tpl := dbgen.Template(t, db, database.Template{
632635
OrganizationID: org.ID,
633636
CreatedBy: owner.ID,
@@ -666,7 +669,22 @@ func TestGetWorkspacesAndAgentsByOwnerID(t *testing.T) {
666669
CreateAgent: false,
667670
})
668671

669-
ownerRows, err := db.GetWorkspacesAndAgentsByOwnerID(ctx, owner.ID)
672+
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
673+
userSubject, _, err := httpmw.UserRBACSubject(ctx, db, user.ID, rbac.ExpandableScope(rbac.ScopeAll))
674+
require.NoError(t, err)
675+
preparedUser, err := authorizer.Prepare(ctx, userSubject, policy.ActionRead, rbac.ResourceWorkspace.Type)
676+
require.NoError(t, err)
677+
userCtx := dbauthz.As(ctx, userSubject)
678+
userRows, err := db.GetAuthorizedWorkspacesAndAgentsByOwnerID(userCtx, owner.ID, preparedUser)
679+
require.NoError(t, err)
680+
require.Len(t, userRows, 0)
681+
682+
ownerSubject, _, err := httpmw.UserRBACSubject(ctx, db, owner.ID, rbac.ExpandableScope(rbac.ScopeAll))
683+
require.NoError(t, err)
684+
preparedOwner, err := authorizer.Prepare(ctx, ownerSubject, policy.ActionRead, rbac.ResourceWorkspace.Type)
685+
require.NoError(t, err)
686+
ownerCtx := dbauthz.As(ctx, ownerSubject)
687+
ownerRows, err := db.GetAuthorizedWorkspacesAndAgentsByOwnerID(ownerCtx, owner.ID, preparedOwner)
670688
require.NoError(t, err)
671689
require.Len(t, ownerRows, 4)
672690
for _, row := range ownerRows {

‎coderd/database/queries.sql.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ WHERE
721721
-- Filter by owner_id
722722
workspaces.owner_id = @owner_id :: uuid
723723
AND workspaces.deleted = false
724+
-- Authorize Filter clause will be injected below in GetAuthorizedWorkspacesAndAgentsByOwnerID
725+
-- @authorize_filter
724726
GROUP BY workspaces.id, workspaces.name, latest_build.job_status, latest_build.job_id, latest_build.transition;
725727

726728

0 commit comments

Comments
 (0)