Skip to content

Commit

Permalink
Update ComputedTrafficPermissions ACL hooks (#20622)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kisunji
kisunji committed Feb 13, 2024
1 parent 64cd172 commit 248969c
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 204 deletions.
10 changes: 8 additions & 2 deletions internal/auth/internal/types/computed_traffic_permissions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,15 @@ func validateComputedTrafficPermissions(res *DecodedComputedTrafficPermissions)
}

func aclReadHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, id *pbresource.ID, _ *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().TrafficPermissionsReadAllowed(id.Name, authzContext)
err := authorizer.ToAllowAuthorizer().TrafficPermissionsReadAllowed(id.Name, authzContext)
if err != nil {
// fallback to operator read
err = authorizer.ToAllowAuthorizer().OperatorReadAllowed(authzContext)
}
return err
}

func aclWriteHookComputedTrafficPermissions(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error {
return authorizer.ToAllowAuthorizer().TrafficPermissionsWriteAllowed(res.Id.Name, authzContext)
// Users should not be writing computed resources.
return authorizer.ToAllowAuthorizer().OperatorWriteAllowed(authzContext)
}
169 changes: 70 additions & 99 deletions internal/auth/internal/types/computed_traffic_permissions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ import (

"github.com/stretchr/testify/require"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest"
pbauth "github.com/hashicorp/consul/proto-public/pbauth/v2beta1"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/sdk/testutil"
)

Expand Down Expand Up @@ -50,127 +47,101 @@ func TestValidateComputedTrafficPermissions_Permissions(t *testing.T) {
}

func TestComputedTrafficPermissionsACLs(t *testing.T) {
// Wire up a registry to generically invoke hooks
registry := resource.NewRegistry()
Register(registry)

type testcase struct {
rules string
check func(t *testing.T, authz acl.Authorizer, res *pbresource.Resource)
readOK string
writeOK string
listOK string
ctpData := &pbauth.ComputedTrafficPermissions{
AllowPermissions: []*pbauth.Permission{},
DenyPermissions: []*pbauth.Permission{},
IsDefault: true,
}

const (
DENY = "deny"
ALLOW = "allow"
DEFAULT = "default"
)

checkF := func(t *testing.T, expect string, got error) {
switch expect {
case ALLOW:
if acl.IsErrPermissionDenied(got) {
t.Fatal("should be allowed")
}
case DENY:
if !acl.IsErrPermissionDenied(got) {
t.Fatal("should be denied")
}
case DEFAULT:
require.Nil(t, got, "expected fallthrough decision")
default:
t.Fatalf("unexpected expectation: %q", expect)
}
}

reg, ok := registry.Resolve(pbauth.ComputedTrafficPermissionsType)
require.True(t, ok)

run := func(t *testing.T, tc testcase) {
ctpData := &pbauth.ComputedTrafficPermissions{}
res := resourcetest.Resource(pbauth.ComputedTrafficPermissionsType, "wi1").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, ctpData).
Build()
resourcetest.ValidateAndNormalize(t, registry, res)

config := acl.Config{
WildcardName: structs.WildcardSpecifier,
}
authz, err := acl.NewAuthorizerFromRules(tc.rules, &config, nil)
require.NoError(t, err)
authz = acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.DenyAll()})

t.Run("read", func(t *testing.T) {
err := reg.ACLs.Read(authz, &acl.AuthorizerContext{}, res.Id, res)
checkF(t, tc.readOK, err)
})
t.Run("write", func(t *testing.T) {
err := reg.ACLs.Write(authz, &acl.AuthorizerContext{}, res)
checkF(t, tc.writeOK, err)
})
t.Run("list", func(t *testing.T) {
err := reg.ACLs.List(authz, &acl.AuthorizerContext{})
checkF(t, tc.listOK, err)
})
}

cases := map[string]testcase{
cases := map[string]resourcetest.ACLTestCase{
"no rules": {
rules: ``,
readOK: DENY,
writeOK: DENY,
listOK: DEFAULT,
Rules: ``,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 read, no intentions": {
rules: `identity "wi1" { policy = "read" }`,
readOK: ALLOW,
writeOK: DENY,
listOK: DEFAULT,
Rules: `identity "test" { policy = "read" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 read, deny intentions": {
rules: `identity "wi1" { policy = "read", intentions = "deny" }`,
readOK: DENY,
writeOK: DENY,
listOK: DEFAULT,
Rules: `identity "test" { policy = "read", intentions = "deny" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 read, intentions read": {
rules: `identity "wi1" { policy = "read", intentions = "read" }`,
readOK: ALLOW,
writeOK: DENY,
listOK: DEFAULT,
Rules: `identity "test" { policy = "read", intentions = "read" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 write, write intentions": {
rules: `identity "wi1" { policy = "read", intentions = "write" }`,
readOK: ALLOW,
writeOK: ALLOW,
listOK: DEFAULT,
Rules: `identity "test" { policy = "read", intentions = "write" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 write, deny intentions": {
rules: `identity "wi1" { policy = "write", intentions = "deny" }`,
readOK: DENY,
writeOK: DENY,
listOK: DEFAULT,
Rules: `identity "test" { policy = "write", intentions = "deny" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.DENY,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 write, intentions read": {
rules: `identity "wi1" { policy = "write", intentions = "read" }`,
readOK: ALLOW,
writeOK: DENY,
listOK: DEFAULT,
Rules: `identity "test" { policy = "write", intentions = "read" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY,
ListOK: resourcetest.DEFAULT,
},
"workload identity w1 write, intentions write": {
rules: `identity "wi1" { policy = "write", intentions = "write" }`,
readOK: ALLOW,
writeOK: ALLOW,
listOK: DEFAULT,
Rules: `identity "test" { policy = "write", intentions = "write" }`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
},
"operator read": {
Rules: `operator = "read"`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.DENY, // users should not write computed resources
ListOK: resourcetest.DEFAULT,
},
"operator write": {
Rules: `operator = "write"`,
Data: ctpData,
Typ: pbauth.ComputedTrafficPermissionsType,
ReadOK: resourcetest.ALLOW,
WriteOK: resourcetest.ALLOW,
ListOK: resourcetest.DEFAULT,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
run(t, tc)
resourcetest.RunACLTestCase(t, tc, registry)
})
}
}

0 comments on commit 248969c

Please sign in to comment.