Skip to content

Commit e30250a

Browse files
aslilacaslilac
committed
refactor: consolidate template and workspace acl validation
1 parent 4ba9382 commit e30250a

File tree

10 files changed

+314
-89
lines changed

10 files changed

+314
-89
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5376,6 +5376,26 @@ func (q *querier) UpsertWorkspaceAppAuditSession(ctx context.Context, arg databa
53765376
return q.db.UpsertWorkspaceAppAuditSession(ctx, arg)
53775377
}
53785378

5379+
func (q *querier) ValidateGroupIDs(ctx context.Context, groupIds []uuid.UUID) (database.ValidateGroupIDsRow, error) {
5380+
// This check is probably overly restrictive, but the "correct" check isn't
5381+
// necessarily obvious. It's only used as a verification check for ACLs right
5382+
// now, which are performed as system.
5383+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
5384+
return database.ValidateGroupIDsRow{}, err
5385+
}
5386+
return q.db.ValidateGroupIDs(ctx, groupIds)
5387+
}
5388+
5389+
func (q *querier) ValidateUserIDs(ctx context.Context, userIds []uuid.UUID) (database.ValidateUserIDsRow, error) {
5390+
// This check is probably overly restrictive, but the "correct" check isn't
5391+
// necessarily obvious. It's only used as a verification check for ACLs right
5392+
// now, which are performed as system.
5393+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
5394+
return database.ValidateUserIDsRow{}, err
5395+
}
5396+
return q.db.ValidateUserIDs(ctx, userIds)
5397+
}
5398+
53795399
func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, _ rbac.PreparedAuthorized) ([]database.Template, error) {
53805400
// TODO Delete this function, all GetTemplates should be authorized. For now just call getTemplates on the authz querier.
53815401
return q.GetTemplatesWithFilter(ctx, arg)

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 14 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: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.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.sql.go

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

‎coderd/database/queries/groups.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,24 @@ WHERE
88
LIMIT
99
1;
1010

11+
-- name: ValidateGroupIDs :one
12+
WITH input AS (
13+
SELECT
14+
unnest(@group_ids::uuid[]) AS id
15+
)
16+
SELECT
17+
array_agg(input.id)::uuid[] as invalid_group_ids,
18+
COUNT(*) = 0 as ok
19+
FROM
20+
-- Preserve rows where there is not a matching left (groups) row for each
21+
-- right (input) row...
22+
users
23+
RIGHT JOIN input ON groups.id = input.id
24+
WHERE
25+
-- ...so that we can retain exactly those rows where an input ID does not
26+
-- match an existing group.
27+
groups.id IS NOT DISTINCT FROM NULL;
28+
1129
-- name: GetGroupByOrgAndName :one
1230
SELECT
1331
*

‎coderd/database/queries/users.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,25 @@ WHERE
2525
LIMIT
2626
1;
2727

28+
-- name: ValidateUserIDs :one
29+
WITH input AS (
30+
SELECT
31+
unnest(@user_ids::uuid[]) AS id
32+
)
33+
SELECT
34+
array_agg(input.id)::uuid[] as invalid_user_ids,
35+
COUNT(*) = 0 as ok
36+
FROM
37+
-- Preserve rows where there is not a matching left (users) row for each
38+
-- right (input) row...
39+
users
40+
RIGHT JOIN input ON users.id = input.id
41+
WHERE
42+
-- ...so that we can retain exactly those rows where an input ID does not
43+
-- match an existing user.
44+
users.id IS NOT DISTINCT FROM NULL AND
45+
users.deleted = false;
46+
2847
-- name: GetUsersByIDs :many
2948
-- This shouldn't check for deleted, because it's frequently used
3049
-- to look up references to actions. eg. a user could build a workspace

‎coderd/rbac/acl/updatevalidator.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package acl
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/google/uuid"
8+
9+
"github.com/coder/coder/v2/coderd/database"
10+
"github.com/coder/coder/v2/coderd/database/dbauthz"
11+
"github.com/coder/coder/v2/codersdk"
12+
)
13+
14+
type ACLUpdateValidator[Role codersdk.WorkspaceRole | codersdk.TemplateRole] interface {
15+
Users() (map[string]Role, string)
16+
Groups() (map[string]Role, string)
17+
ValidateRole(role Role) error
18+
}
19+
20+
func Validate[T codersdk.WorkspaceRole | codersdk.TemplateRole](
21+
ctx context.Context,
22+
db database.Store,
23+
v ACLUpdateValidator[T],
24+
) []codersdk.ValidationError {
25+
// nolint:gocritic // Validate requires full read access to users and groups
26+
ctx = dbauthz.AsSystemRestricted(ctx)
27+
var validErrs []codersdk.ValidationError
28+
29+
groupPerms, groupsField := v.Groups()
30+
groupIDs := make([]uuid.UUID, 0, len(groupPerms))
31+
for idStr, role := range groupPerms {
32+
// Validate the provided role names
33+
if err := v.ValidateRole(role); err != nil {
34+
validErrs = append(validErrs, codersdk.ValidationError{
35+
Field: groupsField,
36+
Detail: err.Error(),
37+
})
38+
}
39+
// Validate that the IDs are UUIDs
40+
id, err := uuid.Parse(idStr)
41+
if err != nil {
42+
validErrs = append(validErrs, codersdk.ValidationError{
43+
Field: groupsField,
44+
Detail: idStr + "is not a valid UUID.",
45+
})
46+
continue
47+
}
48+
groupIDs = append(groupIDs, id)
49+
}
50+
51+
// Validate that the groups exist
52+
groupValidation, err := db.ValidateGroupIDs(ctx, groupIDs)
53+
if err != nil {
54+
validErrs = append(validErrs, codersdk.ValidationError{
55+
Field: groupsField,
56+
Detail: fmt.Sprintf("failed to validate group IDs: %v", err.Error()),
57+
})
58+
}
59+
if !groupValidation.Ok {
60+
for _, id := range groupValidation.InvalidGroupIds {
61+
validErrs = append(validErrs, codersdk.ValidationError{
62+
Field: groupsField,
63+
Detail: fmt.Sprintf("group with ID %v does not exist", id),
64+
})
65+
}
66+
}
67+
68+
userPerms, usersField := v.Users()
69+
userIDs := make([]uuid.UUID, 0, len(userPerms))
70+
for idStr, role := range userPerms {
71+
// Validate the provided role names
72+
if err := v.ValidateRole(role); err != nil {
73+
validErrs = append(validErrs, codersdk.ValidationError{
74+
Field: usersField,
75+
Detail: err.Error(),
76+
})
77+
}
78+
// Validate that the IDs are UUIDs
79+
id, err := uuid.Parse(idStr)
80+
if err != nil {
81+
validErrs = append(validErrs, codersdk.ValidationError{
82+
Field: usersField,
83+
Detail: idStr + "is not a valid UUID.",
84+
})
85+
continue
86+
}
87+
userIDs = append(userIDs, id)
88+
}
89+
90+
// Validate that the groups exist
91+
userValidation, err := db.ValidateUserIDs(ctx, userIDs)
92+
if err != nil {
93+
validErrs = append(validErrs, codersdk.ValidationError{
94+
Field: usersField,
95+
Detail: fmt.Sprintf("failed to validate user IDs: %v", err.Error()),
96+
})
97+
}
98+
if !userValidation.Ok {
99+
for _, id := range userValidation.InvalidUserIds {
100+
validErrs = append(validErrs, codersdk.ValidationError{
101+
Field: usersField,
102+
Detail: fmt.Sprintf("user with ID %v does not exist", id),
103+
})
104+
}
105+
}
106+
107+
return validErrs
108+
}

0 commit comments

Comments
 (0)