acl: add validation to binding rule selector on upsert. (#16210)

* acl: add validation to binding rule selector on upsert.

* docs: add more information on binding rule selector escaping.
This commit is contained in:
James Rasell 2023-02-17 15:38:55 +01:00 committed by GitHub
parent e54f80430d
commit 8295d0e516
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 73 additions and 13 deletions

View File

@ -655,7 +655,6 @@ func TestACLBindingRules(t *testing.T) {
testClient, testServer, _ := makeACLClient(t, nil, nil)
defer testServer.Stop()
//
aclAuthMethod := ACLAuthMethod{
Name: "auth0",
Type: ACLAuthMethodTypeOIDC,
@ -676,7 +675,7 @@ func TestACLBindingRules(t *testing.T) {
bindingRule := ACLBindingRule{
Description: "my-binding-rule",
AuthMethod: "auth0",
Selector: "nomad-engineering-team in list.groups",
Selector: "nomad_engineering_team in list.groups",
BindType: "role",
BindName: "cluster-admin",
}

View File

@ -72,14 +72,14 @@ func TestACLBindingRuleCreateCommand_Run(t *testing.T) {
// Create the ACL binding rule.
args := []string{
"-address=" + url, "-token=" + rootACLToken.SecretID, "-auth-method=acl-binding-rule-cli-test",
"-bind-name=engineering", "-bind-type=role", `-selector="nomad-engineering in list.groups"`,
"-bind-name=engineering", "-bind-type=role", "-selector=engineering in list.groups",
}
must.Eq(t, 0, cmd.Run(args))
s := ui.OutputWriter.String()
must.StrContains(t, s, "acl-binding-rule-cli-test")
must.StrContains(t, s, "role")
must.StrContains(t, s, "engineering")
must.StrContains(t, s, "nomad-engineering in list.groups")
must.StrContains(t, s, "engineering in list.groups")
ui.OutputWriter.Reset()
ui.ErrorWriter.Reset()

View File

@ -9,6 +9,7 @@ import (
"strconv"
"time"
"github.com/hashicorp/go-bexpr"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-set"
lru "github.com/hashicorp/golang-lru/v2"
@ -1146,6 +1147,15 @@ func (a *ACLBindingRule) Validate() error {
mErr.Errors = append(mErr.Errors, fmt.Errorf("unsupported bind type: %q", a.BindType))
}
// If there is a selector configured, ensure that go-bexpr can parse this.
// Otherwise, the user will get an ambiguous failure when attempting to
// login.
if a.Selector != "" {
if _, err := bexpr.CreateEvaluator(a.Selector, nil); err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("selector is invalid: %v", err))
}
}
return mErr.ErrorOrNil()
}

View File

@ -1204,7 +1204,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypePolicy,
BindName: "some-policy-name",
},
@ -1215,7 +1215,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypePolicy,
BindName: "",
},
@ -1230,7 +1230,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypeRole,
BindName: "some-role-name",
},
@ -1241,7 +1241,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypeRole,
BindName: "",
},
@ -1256,7 +1256,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypeManagement,
BindName: "",
},
@ -1267,7 +1267,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
Selector: "group_name in list.groups",
BindType: ACLBindingRuleBindTypeManagement,
BindName: "some-name",
},
@ -1277,6 +1277,21 @@ func TestACLBindingRule_Validate(t *testing.T) {
},
},
},
{
name: "invalid selector",
inputACLBindingRule: &ACLBindingRule{
Description: "some short description",
AuthMethod: "auth0",
Selector: "group-name in list.groups",
BindType: ACLBindingRuleBindTypePolicy,
BindName: "some-policy-name",
},
expectedError: &multierror.Error{
Errors: []error{
errors.New("selector is invalid: 1:6 (5): no match found, expected: \"!=\", \".\", \"==\", \"[\", [ \\t\\r\\n] or [a-zA-Z0-9_/]"),
},
},
},
{
name: "invalid all",
inputACLBindingRule: &ACLBindingRule{
@ -1293,6 +1308,7 @@ func TestACLBindingRule_Validate(t *testing.T) {
errors.New("auth method is missing"),
errors.New("description longer than 256"),
errors.New("bind type is missing"),
errors.New("selector is invalid: 1:6 (5): no match found, expected: \"!=\", \".\", \"==\", \"[\", [ \\t\\r\\n] or [a-zA-Z0-9_/]"),
},
},
},

View File

@ -127,7 +127,11 @@ The table below shows this endpoint's support for
- `Selector` `(string: "")` - A boolean expression that matches against verified
identity attributes returned from the auth method during login. This is
optional and when not set, provides a catch-all rule.
optional and when not set, provides a catch-all rule. If set, it must be a
valid [go-bexpr][] expression; for example, a dash in the claim name will
require it to be encased in quotes and escaped such as
`"\"project-developer\" in list.roles"`.
- `BindType` `(string: <required>)` - Adjusts how this binding rule is applied
at login time. Valid values are `role`, `policy`, and `management`.
@ -200,7 +204,10 @@ queries](/nomad/api-docs#blocking-queries) and [required ACLs](/nomad/api-docs#a
- `Selector` `(string: "")` - A boolean expression that matches against verified
identity attributes returned from the auth method during login. This is
optional and when not set, provides a catch-all rule.
optional and when not set, provides a catch-all rule. If set, it must be a
valid [go-bexpr][] expression; for example, a dash in the claim name will
require it to be encased in quotes and escaped such as
`"\"project-developer\" in list.roles"`.
- `BindType` `(string: "")` - Adjusts how this binding rule is applied at login
time. Valid values are `role`, `policy`, and `management`.
@ -274,3 +281,5 @@ $ curl \
--header "X-Nomad-Token: <NOMAD_TOKEN_SECRET_ID>" \
https://localhost:4646/v1/acl/binding-rule/5da76548-1a60-b8fb-f9be-c7736a5bca09
```
[go-bexpr]: https://github.com/hashicorp/go-bexpr

View File

@ -48,7 +48,12 @@ via flags detailed below.
Create a new ACL Binding Rule:
```shell-session
$ nomad acl binding-rule create -description "example binding rule" -auth-method "auth0" -bind-type "role" -bind-name "eng-ro" -selector "engineering in list.roles"
$ nomad acl binding-rule create \
-description "example binding rule" \
-auth-method "auth0" \
-bind-type "role" \
-bind-name "eng-ro" \
-selector "engineering in list.roles"
ID = 698fdad6-dcb3-79dd-dc72-b43374057dea
Description = example binding rule
Auth Method = auth0
@ -60,3 +65,24 @@ Modify Time = 2022-12-20 11:15:22.582568 +0000 UTC
Create Index = 14
Modify Index = 14
```
Create a new ACL Binding Rule where the selector needs to be escaped:
```shell-session
$ nomad acl binding-rule create \
-description "example binding rule" \
-auth-method "auth0" \
-bind-type "role" \
-bind-name "eng-ro" \
-selector "\"product-developer\" in list.roles"
ID = 698fdad6-dcb3-79dd-dc72-b43374057dea
Description = example binding rule
Auth Method = auth0
Selector = "\"project-developer\" in list.roles"
Bind Type = role
Bind Name = eng-ro
Create Time = 2022-12-20 11:15:22.582568 +0000 UTC
Modify Time = 2022-12-20 11:15:22.582568 +0000 UTC
Create Index = 14
Modify Index = 14
```