Skip to content

chore: Rewrite rbac rego -> SQL clause #5138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Nov 28, 2022
Merged

chore: Rewrite rbac rego -> SQL clause #5138

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d8fce5e
chore: Rewrite rbac rego -> SQL clause
Emyrk Nov 20, 2022
4b2e99f
Begin removing all old code
Emyrk Nov 20, 2022
6c89f1d
Add unit tests for equality and membership
Emyrk Nov 20, 2022
300dc4c
fixup! Add unit tests for equality and membership
Emyrk Nov 20, 2022
87b3454
Rework arguments
Emyrk Nov 20, 2022
de198c5
Add test vectors
Emyrk Nov 20, 2022
b9b1c4e
Handle empty arrays
Emyrk Nov 20, 2022
a0790b0
test: Add unit test verifying acl behavior against all normal cases.
Emyrk Nov 20, 2022
618904e
Implement AlwaysFalsE
Emyrk Nov 20, 2022
5de395a
Make filter ignore owner, not org
Emyrk Nov 21, 2022
e6696cc
Add comment
Emyrk Nov 21, 2022
1f37ca1
Cleanup some code
Emyrk Nov 21, 2022
203bc7c
Cleanup and comments
Emyrk Nov 21, 2022
f0fce00
Linting
Emyrk Nov 21, 2022
a000c15
errors.New -> xerrors.fmt
Emyrk Nov 21, 2022
a521616
More linting
Emyrk Nov 21, 2022
cbb219b
Handle fakeDB for authorize test
Emyrk Nov 21, 2022
f8453af
Add comment
Emyrk Nov 21, 2022
7e1f482
Trying to make the linter happy
Emyrk Nov 21, 2022
7b1231a
Add exempt method
Emyrk Nov 21, 2022
5333401
Merge remote-tracking branch 'origin/main' into stevenmasley/rego_to_…
Emyrk Nov 21, 2022
aae858e
Fix merge with master: Add template fields
Emyrk Nov 21, 2022
38e9fde
Add defensive programming checks
Emyrk Nov 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Linting
  • Loading branch information
@Emyrk
Emyrk committed Nov 21, 2022
commit f0fce003d4a848de88d1a89335b49fe6a265c9d8
1 change: 1 addition & 0 deletions coderd/rbac/regosql/aclGroupVar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (g ACLGroupVar) SQLString(cfg *sqltypes.SQLGenerator) string {
}

func (g ACLGroupVar) ContainsSQL(cfg *sqltypes.SQLGenerator, other sqltypes.Node) (string, error) {
//nolint:singleCaseSwitch
switch other.UseAs().(type) {
// Only supports containing other strings.
case sqltypes.AstString:
Expand Down
1 change: 1 addition & 0 deletions coderd/rbac/regosql/sqltypes/equality.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func (e equality) EqualsSQLString(cfg *SQLGenerator, not bool, other Node) (stri
}

func boolEqualsSQLString(cfg *SQLGenerator, a BooleanNode, not bool, other Node) (string, error) {
//nolint:singleCaseSwitch
switch other.UseAs().(type) {
case BooleanNode:
bn, ok := other.(BooleanNode)
Expand Down
3 changes: 2 additions & 1 deletion coderd/rbac/regosql/sqltypes/number.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ func Number(source RegoSource, v json.Number) Node {

func (AstNumber) UseAs() Node { return AstNumber{} }

func (n AstNumber) SQLString(cfg *SQLGenerator) string {
func (n AstNumber) SQLString(_ *SQLGenerator) string {
return n.Value.String()
}

func (n AstNumber) EqualsSQLString(cfg *SQLGenerator, not bool, other Node) (string, error) {
//nolint:singleCaseSwitch
switch other.UseAs().(type) {
case AstNumber:
return basicSQLEquality(cfg, not, n, other), nil
Expand Down
6 changes: 3 additions & 3 deletions coderd/rbac/regosql/sqltypes/or.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
type binaryOperator int

const (
binaryOpUnknown binaryOperator = iota
_ binaryOperator = iota
binaryOpOR
binaryOpAND
)
Expand Down Expand Up @@ -68,8 +68,8 @@ func (b binaryOp) SQLString(cfg *SQLGenerator) string {

terms := make([]string, 0, len(b.Terms))
for _, term := range b.Terms {
termSql := term.SQLString(cfg)
terms = append(terms, termSql)
termSQL := term.SQLString(cfg)
terms = append(terms, termSQL)
}

return strings.Join(terms, " "+sqlOp+" ")
Expand Down
3 changes: 2 additions & 1 deletion coderd/rbac/regosql/sqltypes/string.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ func String(v string) Node {

func (AstString) UseAs() Node { return AstString{} }

func (s AstString) SQLString(cfg *SQLGenerator) string {
func (s AstString) SQLString(_ *SQLGenerator) string {
return "'" + s.Value + "'"
}

func (s AstString) EqualsSQLString(cfg *SQLGenerator, not bool, other Node) (string, error) {
//nolint:singleCaseSwitch
switch other.UseAs().(type) {
case AstString:
return basicSQLEquality(cfg, not, s, other), nil
Expand Down
3 changes: 2 additions & 1 deletion coderd/rbac/regosql/sqltypes/variable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,12 @@ func (s astStringVar) ConvertVariable(rego ast.Ref) (Node, bool) {
return nil, false
}

func (s astStringVar) SQLString(cfg *SQLGenerator) string {
func (s astStringVar) SQLString(_ *SQLGenerator) string {
return s.ColumnString
}

func (s astStringVar) EqualsSQLString(cfg *SQLGenerator, not bool, other Node) (string, error) {
//nolint:singleCaseSwitch
switch other.UseAs().(type) {
case AstString:
return basicSQLEquality(cfg, not, s, other), nil
Expand Down
11 changes: 7 additions & 4 deletions enterprise/coderd/templates_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ import (
"net/http"
"testing"

"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/cryptorand"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/cryptorand"
"github.com/coder/coder/enterprise/coderd/coderdenttest"
"github.com/coder/coder/provisioner/echo"
"github.com/coder/coder/testutil"
Expand Down Expand Up @@ -895,7 +894,7 @@ func TestTemplateAccess(t *testing.T) {
require.Len(t, exp, 0, "expected templates not found")
}

// Test
// nolint:paralleltest
t.Run("OwnerReadAll", func(t *testing.T) {
for _, o := range orgs {
// Owners can read all templates in all orgs
Expand All @@ -904,6 +903,7 @@ func TestTemplateAccess(t *testing.T) {
}
})

// nolint:paralleltest
t.Run("TemplateAdminReadAll", func(t *testing.T) {
for _, o := range orgs {
// Template Admins can read all templates in all orgs
Expand All @@ -912,6 +912,7 @@ func TestTemplateAccess(t *testing.T) {
}
})

// nolint:paralleltest
t.Run("OrgAdminReadAllTheirs", func(t *testing.T) {
for i, o := range orgs {
cli := o.Admin.Client
Expand All @@ -925,6 +926,7 @@ func TestTemplateAccess(t *testing.T) {
}
})

// nolint:paralleltest
t.Run("TestMemberNoGroup", func(t *testing.T) {
for i, o := range orgs {
cli := o.MemberNoGroup.Client
Expand All @@ -938,6 +940,7 @@ func TestTemplateAccess(t *testing.T) {
}
})

// nolint:paralleltest
t.Run("TestMemberInGroup", func(t *testing.T) {
for i, o := range orgs {
cli := o.MemberInGroup.Client
Expand Down