Skip to content

Commit cd4ab97

Browse files
EmyrkEmyrk
authored
feat: Convert rego queries into SQL clauses (coder#4225)
* feat: Convert rego queries into SQL clauses * Fix postgres quotes to single quotes * Ensure all test cases can compile into SQL clauses * Do not export extra types * Add custom query with rbac filter * First draft of a custom authorized db call * Add comments + tests * Support better regex style matching for variables * Handle jsonb arrays * Remove auth call on workspaces * Fix PG endpoints test * Match psql implementation * Add some comments * Remove unused argument * Add query name for tracking * Handle nested types This solves it without proper types in our AST. Might bite the bullet and implement some better types * Add comment * Renaming function call to GetAuthorizedWorkspaces
1 parent 6325a9e commit cd4ab97

File tree

11 files changed

+870
-20
lines changed

11 files changed

+870
-20
lines changed

‎coderd/authorize.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import (
1313
"github.com/coder/coder/codersdk"
1414
)
1515

16+
// AuthorizeFilter takes a list of objects and returns the filtered list of
17+
// objects that the user is authorized to perform the given action on.
18+
// This is faster than calling Authorize() on each object.
1619
func AuthorizeFilter[O rbac.Objecter](h *HTTPAuthorizer, r *http.Request, action rbac.Action, objects []O) ([]O, error) {
1720
roles := httpmw.UserAuthorization(r)
1821
objects, err := rbac.Filter(r.Context(), h.Authorizer, roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, objects)
@@ -85,6 +88,26 @@ func (h *HTTPAuthorizer) Authorize(r *http.Request, action rbac.Action, object r
8588
return true
8689
}
8790

91+
// AuthorizeSQLFilter returns an authorization filter that can used in a
92+
// SQL 'WHERE' clause. If the filter is used, the resulting rows returned
93+
// from postgres are already authorized, and the caller does not need to
94+
// call 'Authorize()' on the returned objects.
95+
// Note the authorization is only for the given action and object type.
96+
func (h *HTTPAuthorizer) AuthorizeSQLFilter(r *http.Request, action rbac.Action, objectType string) (rbac.AuthorizeFilter, error) {
97+
roles := httpmw.UserAuthorization(r)
98+
prepared, err := h.Authorizer.PrepareByRoleName(r.Context(), roles.ID.String(), roles.Roles, roles.Scope.ToRBAC(), action, objectType)
99+
if err != nil {
100+
return nil, xerrors.Errorf("prepare filter: %w", err)
101+
}
102+
103+
filter, err := prepared.Compile()
104+
if err != nil {
105+
return nil, xerrors.Errorf("compile filter: %w", err)
106+
}
107+
108+
return filter, nil
109+
}
110+
88111
// checkAuthorization returns if the current API key can use the given
89112
// permissions, factoring in the current user's roles and the API key scopes.
90113
func (api *API) checkAuthorization(rw http.ResponseWriter, r *http.Request) {

‎coderd/coderdtest/authorize.go

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,6 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
124124
AssertAction: rbac.ActionCreate,
125125
AssertObject: workspaceExecObj,
126126
},
127-
"GET:/api/v2/workspaces/": {
128-
StatusCode: http.StatusOK,
129-
AssertAction: rbac.ActionRead,
130-
AssertObject: workspaceRBACObj,
131-
},
132127
"GET:/api/v2/organizations/{organization}/templates": {
133128
StatusCode: http.StatusOK,
134129
AssertAction: rbac.ActionRead,
@@ -246,6 +241,9 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
246241
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
247242
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
248243
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
244+
245+
// Endpoints that use the SQLQuery filter.
246+
"GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true},
249247
}
250248

251249
// Routes like proxy routes support all HTTP methods. A helper func to expand
@@ -513,6 +511,12 @@ type RecordingAuthorizer struct {
513511

514512
var _ rbac.Authorizer = (*RecordingAuthorizer)(nil)
515513

514+
// ByRoleNameSQL does not record the call. This matches the postgres behavior
515+
// of not calling Authorize()
516+
func (r *RecordingAuthorizer) ByRoleNameSQL(_ context.Context, _ string, _ []string, _ rbac.Scope, _ rbac.Action, _ rbac.Object) error {
517+
return r.AlwaysReturn
518+
}
519+
516520
func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, roleNames []string, scope rbac.Scope, action rbac.Action, object rbac.Object) error {
517521
r.Called = &authCall{
518522
SubjectID: subjectID,
@@ -526,11 +530,12 @@ func (r *RecordingAuthorizer) ByRoleName(_ context.Context, subjectID string, ro
526530

527531
func (r *RecordingAuthorizer) PrepareByRoleName(_ context.Context, subjectID string, roles []string, scope rbac.Scope, action rbac.Action, _ string) (rbac.PreparedAuthorized, error) {
528532
return &fakePreparedAuthorizer{
529-
Original: r,
530-
SubjectID: subjectID,
531-
Roles: roles,
532-
Scope: scope,
533-
Action: action,
533+
Original: r,
534+
SubjectID: subjectID,
535+
Roles: roles,
536+
Scope: scope,
537+
Action: action,
538+
HardCodedSQLString: "true",
534539
}, nil
535540
}
536541

@@ -539,13 +544,39 @@ func (r *RecordingAuthorizer) reset() {
539544
}
540545

541546
type fakePreparedAuthorizer struct {
542-
Original *RecordingAuthorizer
543-
SubjectID string
544-
Roles []string
545-
Scope rbac.Scope
546-
Action rbac.Action
547+
Original *RecordingAuthorizer
548+
SubjectID string
549+
Roles []string
550+
Scope rbac.Scope
551+
Action rbac.Action
552+
HardCodedSQLString string
553+
HardCodedRegoString string
547554
}
548555

549556
func (f *fakePreparedAuthorizer) Authorize(ctx context.Context, object rbac.Object) error {
550557
return f.Original.ByRoleName(ctx, f.SubjectID, f.Roles, f.Scope, f.Action, object)
551558
}
559+
560+
// Compile returns a compiled version of the authorizer that will work for
561+
// in memory databases. This fake version will not work against a SQL database.
562+
func (f *fakePreparedAuthorizer) Compile() (rbac.AuthorizeFilter, error) {
563+
return f, nil
564+
}
565+
566+
func (f *fakePreparedAuthorizer) Eval(object rbac.Object) bool {
567+
return f.Original.ByRoleNameSQL(context.Background(), f.SubjectID, f.Roles, f.Scope, f.Action, object) == nil
568+
}
569+
570+
func (f fakePreparedAuthorizer) RegoString() string {
571+
if f.HardCodedRegoString != "" {
572+
return f.HardCodedRegoString
573+
}
574+
panic("not implemented")
575+
}
576+
577+
func (f fakePreparedAuthorizer) SQLString(_ rbac.SQLConfig) string {
578+
if f.HardCodedSQLString != "" {
579+
return f.HardCodedSQLString
580+
}
581+
panic("not implemented")
582+
}

‎coderd/database/custom_queries.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package database
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/lib/pq"
8+
"golang.org/x/xerrors"
9+
10+
"github.com/coder/coder/coderd/rbac"
11+
)
12+
13+
type customQuerier interface {
14+
GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error)
15+
}
16+
17+
// GetAuthorizedWorkspaces returns all workspaces that the user is authorized to access.
18+
// This code is copied from `GetWorkspaces` and adds the authorized filter WHERE
19+
// clause.
20+
func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) {
21+
// The name comment is for metric tracking
22+
query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s AND %s", getWorkspaces, authorizedFilter.SQLString(rbac.DefaultConfig()))
23+
rows, err := q.db.QueryContext(ctx, query,
24+
arg.Deleted,
25+
arg.OwnerID,
26+
arg.OwnerUsername,
27+
arg.TemplateName,
28+
pq.Array(arg.TemplateIds),
29+
arg.Name,
30+
)
31+
if err != nil {
32+
return nil, xerrors.Errorf("get authorized workspaces: %w", err)
33+
}
34+
defer rows.Close()
35+
var items []Workspace
36+
for rows.Next() {
37+
var i Workspace
38+
if err := rows.Scan(
39+
&i.ID,
40+
&i.CreatedAt,
41+
&i.UpdatedAt,
42+
&i.OwnerID,
43+
&i.OrganizationID,
44+
&i.TemplateID,
45+
&i.Deleted,
46+
&i.Name,
47+
&i.AutostartSchedule,
48+
&i.Ttl,
49+
&i.LastUsedAt,
50+
); err != nil {
51+
return nil, err
52+
}
53+
items = append(items, i)
54+
}
55+
if err := rows.Close(); err != nil {
56+
return nil, err
57+
}
58+
if err := rows.Err(); err != nil {
59+
return nil, err
60+
}
61+
return items, nil
62+
}

‎coderd/database/databasefake/databasefake.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,13 @@ func (q *fakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U
520520
}, nil
521521
}
522522

523-
func (q *fakeQuerier) GetWorkspaces(_ context.Context, arg database.GetWorkspacesParams) ([]database.Workspace, error) {
523+
func (q *fakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.Workspace, error) {
524+
// A nil auth filter means no auth filter.
525+
workspaces, err := q.GetAuthorizedWorkspaces(ctx, arg, nil)
526+
return workspaces, err
527+
}
528+
529+
func (q *fakeQuerier) GetAuthorizedWorkspaces(_ context.Context, arg database.GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]database.Workspace, error) {
524530
q.mutex.RLock()
525531
defer q.mutex.RUnlock()
526532

@@ -560,6 +566,11 @@ func (q *fakeQuerier) GetWorkspaces(_ context.Context, arg database.GetWorkspace
560566
continue
561567
}
562568
}
569+
570+
// If the filter exists, ensure the object is authorized.
571+
if authorizedFilter != nil && !authorizedFilter.Eval(workspace.RBACObject()) {
572+
continue
573+
}
563574
workspaces = append(workspaces, workspace)
564575
}
565576

‎coderd/database/db.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
// It extends the generated interface to add transaction support.
2121
type Store interface {
2222
querier
23+
// customQuerier contains custom queries that are not generated.
24+
customQuerier
2325

2426
InTx(func(Store) error) error
2527
}

‎coderd/rbac/authz.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Authorizer interface {
2121

2222
type PreparedAuthorized interface {
2323
Authorize(ctx context.Context, object Object) error
24+
Compile() (AuthorizeFilter, error)
2425
}
2526

2627
// Filter takes in a list of objects, and will filter the list removing all

‎coderd/rbac/authz_internal_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,11 @@ func testAuthorize(t *testing.T, name string, subject subject, sets ...[]authTes
781781
partialAuthz, err := authorizer.Prepare(ctx, subject.UserID, subject.Roles, subject.Scope, a, c.resource.Type)
782782
require.NoError(t, err, "make prepared authorizer")
783783

784+
// Ensure the partial can compile to a SQL clause.
785+
// This does not guarantee that the clause is valid SQL.
786+
_, err = Compile(partialAuthz.partialQueries)
787+
require.NoError(t, err, "compile prepared authorizer")
788+
784789
// Also check the rego policy can form a valid partial query result.
785790
// This ensures we can convert the queries into SQL WHERE clauses in the future.
786791
// If this function returns 'Support' sections, then we cannot convert the query into SQL.

‎coderd/rbac/partial.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ type PartialAuthorizer struct {
2828

2929
var _ PreparedAuthorized = (*PartialAuthorizer)(nil)
3030

31+
func (pa *PartialAuthorizer) Compile() (AuthorizeFilter, error) {
32+
filter, err := Compile(pa.partialQueries)
33+
if err != nil {
34+
return nil, xerrors.Errorf("compile: %w", err)
35+
}
36+
return filter, nil
37+
}
38+
3139
func (pa *PartialAuthorizer) Authorize(ctx context.Context, object Object) error {
3240
if pa.alwaysTrue {
3341
return nil

0 commit comments

Comments
 (0)