feat: Convert rego queries into SQL clauses #4225

Emyrk · 2022-09-27T17:37:30Z

What this does

Currently Filter works by querying all resources of a given type, then calling Authorize() on each element in the list. This would not work with pagination.

This feature takes the queries resulting from partial execution, and returns a SQL clause that can be added into a WHERE statement of a SQL query. The results are equivalent to all authorized objects in the original Filter call.

This PR only implements this for listing workspaces. This technique can be copied easily to other listing (templates, users, etc). I just started with the one with the largest lists and the one we will likely paginate first.

Example

Partial execution returns these queries. If either is true, the user in question can read the object.

+---------+--------------------------------------------------------------------+
| Query 1 | input.object.org_owner != ""                                       |
|         | input.object.org_owner in {"19a3d781-9e53-4608-948e-ac084470cd99"} |
|         | input.object.owner != ""                                           |
|         | "me" = input.object.owner                                          |
+---------+--------------------------------------------------------------------+
| Query 2 | input.object.org_owner = ""                                        |
|         | input.object.owner != ""                                           |
|         | "me" = input.object.owner                                          |
+---------+--------------------------------------------------------------------+

This code converts these rego queries into the following SQL:

SELECT * FROM workspaces WHERE
-- SQL generated below this line
(
    (
        input.object.org_owner != '' AND 
        input.object.org_owner = ANY(ARRAY ['a207e4ff-2cf4-44be-aadd-8f866452e3b1']) AND 
        input.object.owner != '' AND 'me' = input.object.owner
    ) 
        OR
    (
        input.object.org_owner = '' AND 
        input.object.owner != '' AND 
        'me' = input.object.owner
    )
)

Long Rego Query Example

+---------+--------------------------------------------------------------------+
| Query 1 | input.object.org_owner != ""                                       |
|         | input.object.org_owner in {"19a3d781-9e53-4608-948e-ac084470cd99"} |
|         | input.object.owner != ""                                           |
|         | "me" = input.object.owner                                          |
+---------+--------------------------------------------------------------------+
| Query 2 | input.object.org_owner = ""                                        |
|         | input.object.owner != ""                                           |
|         | "me" = input.object.owner                                          |
+---------+--------------------------------------------------------------------+
| Query 3 | input.object.org_owner != ""                                       |
|         | input.object.org_owner in {"19a3d781-9e53-4608-948e-ac084470cd99"} |
|         | input.object.org_owner != ""                                       |
|         | "read" in input.object.acl_group_list.allUsers                     |
+---------+--------------------------------------------------------------------+
| Query 4 | input.object.org_owner != ""                                       |
|         | input.object.org_owner in {"19a3d781-9e53-4608-948e-ac084470cd99"} |
|         | input.object.org_owner != ""                                       |
|         | "*" in input.object.acl_group_list.allUsers                        |
+---------+--------------------------------------------------------------------+
| Query 5 | input.object.org_owner = ""                                        |
|         | input.object.org_owner != ""                                       |
|         | "read" in input.object.acl_group_list.allUsers                     |
+---------+--------------------------------------------------------------------+
| Query 6 | input.object.org_owner = ""                                        |
|         | input.object.org_owner != ""                                       |
|         | "*" in input.object.acl_group_list.allUsers                        |
+---------+--------------------------------------------------------------------+
| Query 7 | "read" in input.object.acl_user_list.me                            |
+---------+--------------------------------------------------------------------+
| Query 8 | "*" in input.object.acl_user_list.me                               |
**+---------+--------------------------------------------------------------------+**

Future Work

Implement this technique for other list calls.

Emyrk · 2022-09-28T18:39:20Z

coderd/database/custom_queries.go

+)
+
+// AuthorizedGetWorkspaces returns all workspaces that the user is authorized to access.
+func (q *sqlQuerier) AuthorizedGetWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) {


I wish we could add hooks into SQLc to do this. Kinda sucks to have to copy the original function and call it like this.

coderd/database/custom_queries.go

johnstcn

This looks good to me, but I'll defer to other folks who are working on v2.

This solves it without proper types in our AST. Might bite the bullet and implement some better types

kylecarbs

These tests are surprisingly simple to follow, or at least much simpler than I would have expected interfacing with Rego to be.

My primary concern is the query separation between generated and non-generated. I'd love if we could have a single query (maybe even for just getting a single?) that used the same authorization flow, but maybe that's just unreasonable.

eg.

GetWorkspaceByID
GetWorkspaces
GetWorkspaceByOwnerAndName

Could all be replaced by this?

kylecarbs · 2022-09-30T15:16:13Z

coderd/database/custom_queries.go

+// AuthorizedGetWorkspaces returns all workspaces that the user is authorized to access.
+// This code is copied from `GetWorkspaces` and adds the authorized filter WHERE
+// clause.
+func (q *sqlQuerier) AuthorizedGetWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) {


I might change the wording of this. Prefixing with Authorized makes me think that I'm authorized to get workspaces, not that the workspaces returned I'm authorized to access.

Thoughts on GetAuthorizedWorkspaces instead?

Is it possible for us to get rid of GetWorkspaces in its entirety? There doesn't seem much of a point in getting workspaces that the user doesn't have access to.

@kylecarbs that is a good question. For system level things there might be a need? But we can always add back "GetWorkspaces" later.

I was using SQLc's generated code to make this function. To get rid of GetWorkspaces would be to remove all those SQLc calls. I guess that is ok? It is nice to use it to generate most of this code, it just can't support the dynamic parts of the query.

Emyrk · 2022-09-30T16:19:16Z

AuthorizedGetWorkspaces

This change is only made for "listing" objects, for singular objects running the rego on the returned object is easier. So if we implemented this for the singular too, I would not convert to SQL, I would just call the sqlc code and run Authorize() on that returned object.

I wish there was a way to integrate this into SQLc, as it is a great tool, it just doesn't support this use case.

My goal for this PR was to implement this functionality to make this possible. We next implement it for the 2/3 list queries that are "slow" with the Filter. It is really only needed in 2/3 places to support that dashboard "List all that I can view" view.

Emyrk added 7 commits September 27, 2022 13:25

feat: Convert rego queries into SQL clauses

ff931dc

Fix postgres quotes to single quotes

e535e5a

Ensure all test cases can compile into SQL clauses

8f92953

Do not export extra types

cb5d519

Add custom query with rbac filter

2bd0165

First draft of a custom authorized db call

364498c

Merge remote-tracking branch 'origin/main' into stevenmasley/rego_to_sql

b2da580

Emyrk commented Sep 28, 2022

View reviewed changes

Emyrk added 6 commits September 28, 2022 15:00

Add comments + tests

d516be7

Support better regex style matching for variables

125931a

Handle jsonb arrays

fc58da5

Remove auth call on workspaces

04c1d6a

Fix PG endpoints test

98b405e

Match psql implementation

6ad0b51

Emyrk marked this pull request as ready for review September 29, 2022 00:37

Emyrk added 3 commits September 28, 2022 20:52

Add some comments

7cfad87

Merge remote-tracking branch 'origin/main' into stevenmasley/rego_to_sql

33c9d34

Remove unused argument

f10e9b7

Emyrk requested a review from kylecarbs September 29, 2022 13:24

Emyrk commented Sep 29, 2022

View reviewed changes

coderd/database/custom_queries.go Outdated Show resolved Hide resolved

johnstcn reviewed Sep 29, 2022

View reviewed changes

Emyrk added 3 commits September 29, 2022 15:07

Add query name for tracking

d89c4d2

Handle nested types

3828c29

This solves it without proper types in our AST. Might bite the bullet and implement some better types

Add comment

913fb27

kylecarbs reviewed Sep 30, 2022

View reviewed changes

Renaming function call to GetAuthorizedWorkspaces

3e2fbb8

kylecarbs approved these changes Oct 4, 2022

View reviewed changes

Emyrk merged commit cd4ab97 into main Oct 4, 2022

Emyrk deleted the stevenmasley/rego_to_sql branch October 4, 2022 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Convert rego queries into SQL clauses #4225

feat: Convert rego queries into SQL clauses #4225

Uh oh!

Emyrk commented Sep 27, 2022 •

edited

Loading

Uh oh!

Emyrk Sep 28, 2022

Uh oh!

Uh oh!

johnstcn left a comment

Uh oh!

kylecarbs left a comment

Uh oh!

kylecarbs Sep 30, 2022

Uh oh!

kylecarbs Sep 30, 2022

Uh oh!

Emyrk Sep 30, 2022

Uh oh!

Emyrk commented Sep 30, 2022

Uh oh!

Uh oh!

feat: Convert rego queries into SQL clauses #4225

feat: Convert rego queries into SQL clauses #4225

Uh oh!

Conversation

Emyrk commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

Example

Long Rego Query Example

Future Work

Uh oh!

Emyrk Sep 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

kylecarbs left a comment

Choose a reason for hiding this comment

Uh oh!

kylecarbs Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

kylecarbs Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

Emyrk Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

Emyrk commented Sep 30, 2022

Uh oh!

Uh oh!

Emyrk commented Sep 27, 2022 •

edited

Loading