Skip to content

Commit

Permalink
[NET-6640] Adds "Policy" BindType to BindingRule (hashicorp#19499)
Browse files Browse the repository at this point in the history
feat: add bind type of policy

Co-authored-by: Ronald Ekambi <ronekambi@gmail.com>
  • Loading branch information
2 people authored and Lord-Y committed Dec 14, 2023
1 parent 3d81f74 commit 3c2673d
Show file tree
Hide file tree
Showing 12 changed files with 262 additions and 26 deletions.
3 changes: 3 additions & 0 deletions .changelog/19499.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
acl: add policy bindtype to binding rules.
```
4 changes: 4 additions & 0 deletions acl/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func IsValidRoleName(name string) bool {
return validRoleName.MatchString(name)
}

func IsValidPolicyName(name string) bool {
return ValidatePolicyName(name) == nil
}

// IsValidRoleName returns true if the provided name can be used as an
// ACLAuthMethod Name.
func IsValidAuthMethodName(name string) bool {
Expand Down
47 changes: 46 additions & 1 deletion agent/consul/acl_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3663,6 +3663,37 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) {
require.Equal(t, "test-node", rule.BindName)
})

t.Run("Bind Policy", func(t *testing.T) {
req := structs.ACLBindingRuleSetRequest{
Datacenter: "dc1",
BindingRule: structs.ACLBindingRule{
Description: "foobar policy",
AuthMethod: testAuthMethod.Name,
Selector: "serviceaccount.name==abc",
BindType: structs.BindingRuleBindTypePolicy,
BindName: "test-policy",
},
WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken},
}
var resp structs.ACLBindingRule

err := aclEp.BindingRuleSet(&req, &resp)
require.NoError(t, err)
require.NotNil(t, resp.ID)

// Get the rule directly to validate that it exists
ruleResp, err := retrieveTestBindingRule(codec, TestDefaultInitialManagementToken, "dc1", resp.ID)
require.NoError(t, err)
rule := ruleResp.BindingRule

require.NotEmpty(t, rule.ID)
require.Equal(t, rule.Description, "foobar policy")
require.Equal(t, rule.AuthMethod, testAuthMethod.Name)
require.Equal(t, "serviceaccount.name==abc", rule.Selector)
require.Equal(t, structs.BindingRuleBindTypePolicy, rule.BindType)
require.Equal(t, "test-policy", rule.BindName)
})

t.Run("templated policy", func(t *testing.T) {
req := structs.ACLBindingRuleSetRequest{
Datacenter: "dc1",
Expand Down Expand Up @@ -3841,7 +3872,7 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) {
t.Run("Create fails; invalid bind type", func(t *testing.T) {
reqRule := newRule()
reqRule.BindType = "invalid"
requireSetErrors(t, reqRule, "Invalid Binding Rule: unknown BindType")
requireSetErrors(t, reqRule, "invalid Binding Rule: unknown BindType")
})

t.Run("Create fails; bind name with unknown vars", func(t *testing.T) {
Expand Down Expand Up @@ -4540,6 +4571,11 @@ func TestACLEndpoint_Login(t *testing.T) {
"fake-node",
"default", "mynode", "jkl101",
)
testauth.InstallSessionToken(
testSessionID,
"fake-policy", // 1 rule (policy)
"default", "mypolicy", "jkl012",
)

method, err := upsertTestAuthMethod(codec, TestDefaultInitialManagementToken, "dc1", testSessionID)
require.NoError(t, err)
Expand Down Expand Up @@ -4587,6 +4623,15 @@ func TestACLEndpoint_Login(t *testing.T) {
)
require.NoError(t, err)

// policy rule
_, err = upsertTestBindingRule(
codec, TestDefaultInitialManagementToken, "dc1", method.Name,
"serviceaccount.namespace==default and serviceaccount.name==mypolicy",
structs.BindingRuleBindTypePolicy,
"method-${serviceaccount.name}",
)
require.NoError(t, err)

t.Run("do not provide a token", func(t *testing.T) {
req := structs.ACLLoginRequest{
Auth: &structs.ACLLoginParams{
Expand Down
32 changes: 29 additions & 3 deletions agent/consul/auth/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,16 @@ func NewBinder(store BinderStateStore, datacenter string) *Binder {
type BinderStateStore interface {
ACLBindingRuleList(ws memdb.WatchSet, methodName string, entMeta *acl.EnterpriseMeta) (uint64, structs.ACLBindingRules, error)
ACLRoleGetByName(ws memdb.WatchSet, roleName string, entMeta *acl.EnterpriseMeta) (uint64, *structs.ACLRole, error)
ACLPolicyGetByName(ws memdb.WatchSet, policyName string, entMeta *acl.EnterpriseMeta) (uint64, *structs.ACLPolicy, error)
}

// Bindings contains the ACL roles, service identities, node identities,
// Bindings contains the ACL roles, service identities, node identities, policies,
// templated policies, and enterprise meta to be assigned to the created token.
type Bindings struct {
Roles []structs.ACLTokenRoleLink
ServiceIdentities []*structs.ACLServiceIdentity
NodeIdentities []*structs.ACLNodeIdentity
Policies []structs.ACLTokenPolicyLink
TemplatedPolicies structs.ACLTemplatedPolicies
EnterpriseMeta acl.EnterpriseMeta
}
Expand All @@ -58,7 +60,8 @@ func (b *Bindings) None() bool {
return len(b.ServiceIdentities) == 0 &&
len(b.NodeIdentities) == 0 &&
len(b.TemplatedPolicies) == 0 &&
len(b.Roles) == 0
len(b.Roles) == 0 &&
len(b.Policies) == 0
}

// Bind collects the ACL roles, service identities, etc. to be assigned to the
Expand Down Expand Up @@ -119,6 +122,24 @@ func (b *Binder) Bind(authMethod *structs.ACLAuthMethod, verifiedIdentity *authm
}
bindings.TemplatedPolicies = append(bindings.TemplatedPolicies, templatedPolicy)

case structs.BindingRuleBindTypePolicy:
bindName, err := computeBindName(rule.BindName, verifiedIdentity.ProjectedVars, acl.IsValidRoleName)
if err != nil {
return nil, err
}

_, policy, err := b.store.ACLPolicyGetByName(nil, bindName, &bindings.EnterpriseMeta)
if err != nil {
return nil, err
}

if policy != nil {
bindings.Policies = append(bindings.Policies, structs.ACLTokenPolicyLink{
ID: policy.ID,
Name: policy.Name,
})
}

case structs.BindingRuleBindTypeRole:
bindName, err := computeBindName(rule.BindName, verifiedIdentity.ProjectedVars, acl.IsValidRoleName)
if err != nil {
Expand Down Expand Up @@ -177,8 +198,13 @@ func IsValidBindingRule(bindType, bindName string, bindVars *structs.ACLTemplate
if _, err := computeBindName(bindName, fakeVarMap, acl.IsValidRoleName); err != nil {
return fmt.Errorf("failed to validate bindType %q: %w", bindType, err)
}

case structs.BindingRuleBindTypePolicy:
if _, err := computeBindName(bindName, fakeVarMap, acl.IsValidPolicyName); err != nil {
return fmt.Errorf("failed to validate bindType %q: %w", bindType, err)
}
default:
return fmt.Errorf("Invalid Binding Rule: unknown BindType %q", bindType)
return fmt.Errorf("invalid Binding Rule: unknown BindType %q", bindType)
}

return nil
Expand Down
143 changes: 124 additions & 19 deletions agent/consul/auth/binder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/hashicorp/consul/agent/consul/authmethod"
"github.com/hashicorp/consul/agent/consul/state"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
)

func TestBindings_None(t *testing.T) {
Expand All @@ -27,11 +28,79 @@ func TestBindings_None(t *testing.T) {
b = &Bindings{Roles: []structs.ACLTokenRoleLink{{ID: generateID(t)}}}
require.False(t, b.None())

b = &Bindings{Policies: []structs.ACLTokenPolicyLink{{ID: generateID(t)}}}
require.False(t, b.None())

b = &Bindings{ServiceIdentities: []*structs.ACLServiceIdentity{{ServiceName: "web"}}}
require.False(t, b.None())

b = &Bindings{NodeIdentities: []*structs.ACLNodeIdentity{{NodeName: "node-123"}}}
require.False(t, b.None())

b = &Bindings{TemplatedPolicies: []*structs.ACLTemplatedPolicy{{TemplateName: api.ACLTemplatedPolicyDNSName}}}
require.False(t, b.None())
}

func TestBinder_Policy_Success(t *testing.T) {
store := testStateStore(t)
binder := &Binder{store: store}

authMethod := &structs.ACLAuthMethod{
Name: "test-auth-method",
Type: "testing",
}
require.NoError(t, store.ACLAuthMethodSet(0, authMethod))

targetPolicy := &structs.ACLPolicy{
ID: generateID(t),
Name: "foo-policy",
}
require.NoError(t, store.ACLPolicySet(0, targetPolicy))

otherPolicy := &structs.ACLPolicy{
ID: generateID(t),
Name: "not-my-policy",
}
require.NoError(t, store.ACLPolicySet(0, otherPolicy))

bindingRules := structs.ACLBindingRules{
{
ID: generateID(t),
Selector: "role==engineer",
BindType: structs.BindingRuleBindTypePolicy,
BindName: "${editor}-policy",
AuthMethod: authMethod.Name,
},
{
ID: generateID(t),
Selector: "role==engineer",
BindType: structs.BindingRuleBindTypePolicy,
BindName: "this-policy-does-not-exist",
AuthMethod: authMethod.Name,
},
{
ID: generateID(t),
Selector: "language==js",
BindType: structs.BindingRuleBindTypePolicy,
BindName: otherPolicy.Name,
AuthMethod: authMethod.Name,
},
}
require.NoError(t, store.ACLBindingRuleBatchSet(0, bindingRules))

result, err := binder.Bind(&structs.ACLAuthMethod{}, &authmethod.Identity{
SelectableFields: map[string]string{
"role": "engineer",
"language": "go",
},
ProjectedVars: map[string]string{
"editor": "foo",
},
})
require.NoError(t, err)
require.Equal(t, []structs.ACLTokenPolicyLink{
{ID: targetPolicy.ID, Name: targetPolicy.Name},
}, result.Policies)
}

func TestBinder_Roles_Success(t *testing.T) {
Expand Down Expand Up @@ -122,6 +191,32 @@ func TestBinder_Roles_NameValidation(t *testing.T) {
require.Contains(t, err.Error(), "invalid bind name")
}

func TestBinder_Policy_NameValidation(t *testing.T) {
store := testStateStore(t)
binder := &Binder{store: store}

authMethod := &structs.ACLAuthMethod{
Name: "test-auth-method",
Type: "testing",
}
require.NoError(t, store.ACLAuthMethodSet(0, authMethod))

bindingRules := structs.ACLBindingRules{
{
ID: generateID(t),
Selector: "",
BindType: structs.BindingRuleBindTypePolicy,
BindName: "INVALID!",
AuthMethod: authMethod.Name,
},
}
require.NoError(t, store.ACLBindingRuleBatchSet(0, bindingRules))

_, err := binder.Bind(&structs.ACLAuthMethod{}, &authmethod.Identity{})
require.Error(t, err)
require.Contains(t, err.Error(), "invalid bind name")
}

func TestBinder_ServiceIdentities_Success(t *testing.T) {
store := testStateStore(t)
binder := &Binder{store: store}
Expand Down Expand Up @@ -275,54 +370,60 @@ func Test_IsValidBindingRule(t *testing.T) {
"invalid", "blah", nil, "", true},
// valid HIL, invalid name
{"empty",
"both", "", nil, "", true},
"all", "", nil, "", true},
{"just end",
"both", "}", nil, "", true},
"all", "}", nil, "", true},
{"var without start",
"both", " item }", nil, "item", true},
"all", " item }", nil, "item", true},
{"two vars missing second start",
"both", "before-${ item }after--more }", nil, "item,more", true},
"all", "before-${ item }after--more }", nil, "item,more", true},
// names for the two types are validated differently
{"@ is disallowed",
"both", "bad@name", nil, "", true},
"all", "bad@name", nil, "", true},
{"leading dash",
"role", "-name", nil, "", false},
{"leading dash",
"policy", "-name", nil, "", false},
{"leading dash",
"service", "-name", nil, "", true},
{"trailing dash",
"role", "name-", nil, "", false},
{"trailing dash",
"policy", "name-", nil, "", false},
{"trailing dash",
"service", "name-", nil, "", true},
{"inner dash",
"both", "name-end", nil, "", false},
"all", "name-end", nil, "", false},
{"upper case",
"role", "NAME", nil, "", false},
{"upper case",
"policy", "NAME", nil, "", false},
{"upper case",
"service", "NAME", nil, "", true},
// valid HIL, valid name
{"no vars",
"both", "nothing", nil, "", false},
"all", "nothing", nil, "", false},
{"just var",
"both", "${item}", nil, "item", false},
"all", "${item}", nil, "item", false},
{"var in middle",
"both", "before-${item}after", nil, "item", false},
"all", "before-${item}after", nil, "item", false},
{"two vars",
"both", "before-${item}after-${more}", nil, "item,more", false},
"all", "before-${item}after-${more}", nil, "item,more", false},
// bad
{"no bind name",
"both", "", nil, "", true},
"all", "", nil, "", true},
{"just start",
"both", "${", nil, "", true},
"all", "${", nil, "", true},
{"backwards",
"both", "}${", nil, "", true},
"all", "}${", nil, "", true},
{"no varname",
"both", "${}", nil, "", true},
"all", "${}", nil, "", true},
{"missing map key",
"both", "${item}", nil, "", true},
"all", "${item}", nil, "", true},
{"var without end",
"both", "${ item ", nil, "item", true},
"all", "${ item ", nil, "item", true},
{"two vars missing first end",
"both", "before-${ item after-${ more }", nil, "item,more", true},
"all", "before-${ item after-${ more }", nil, "item,more", true},

// bind type: templated policy - bad input
{"templated-policy missing bindvars", "templated-policy", "builtin/service", nil, "", true},
Expand All @@ -338,12 +439,16 @@ func Test_IsValidBindingRule(t *testing.T) {
"templated-policy", "builtin/service", &structs.ACLTemplatedPolicyVariables{Name: "before-${item}after-${more}"}, "item,more", false},
} {
var cases []testcase
if test.bindType == "both" {
if test.bindType == "all" {
test1 := test
test1.bindType = "role"
test2 := test
test2.bindType = "service"
cases = []testcase{test1, test2}
test3 := test
test3.bindType = "policy"
test4 := test
test4.bindType = "node"
cases = []testcase{test1, test2, test3, test4}
} else {
cases = []testcase{test}
}
Expand Down
1 change: 1 addition & 0 deletions agent/consul/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (l *Login) TokenForVerifiedIdentity(identity *authmethod.Identity, authMeth
NodeIdentities: bindings.NodeIdentities,
TemplatedPolicies: bindings.TemplatedPolicies,
Roles: bindings.Roles,
Policies: bindings.Policies,
EnterpriseMeta: bindings.EnterpriseMeta,
}
token.ACLAuthMethodEnterpriseMeta.FillWithEnterpriseMeta(&authMethod.EnterpriseMeta)
Expand Down
Loading

0 comments on commit 3c2673d

Please sign in to comment.