Skip to content
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.

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

Add support for returning ACL secret IDs for accessors with acl:write #10546

Merged
merged 7 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/10546.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
acl: Return secret ID when listing tokens if accessor has `acl:write`
```
1 change: 1 addition & 0 deletions agent/acl_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ func TestACL_HTTP(t *testing.T) {
found := false
for _, actual := range tokens {
if actual.AccessorID == tokenID {
require.Equal(t, expected.SecretID, actual.SecretID)
require.Equal(t, expected.Description, actual.Description)
require.Equal(t, expected.Policies, actual.Policies)
require.Equal(t, expected.Local, actual.Local)
Expand Down
5 changes: 5 additions & 0 deletions agent/consul/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,11 @@ func (f *aclFilter) filterTokenStub(token **structs.ACLTokenListStub) {

if f.authorizer.ACLRead(&entCtx) != acl.Allow {
*token = nil
} else if f.authorizer.ACLWrite(&entCtx) != acl.Allow {
// no write permissions - redact secret
clone := *(*token)
clone.SecretID = redactedToken
*token = &clone
}
}

Expand Down
30 changes: 30 additions & 0 deletions agent/consul/acl_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2066,6 +2066,36 @@ func TestACLEndpoint_TokenList(t *testing.T) {
}
require.ElementsMatch(t, gatherIDs(t, resp.Tokens), tokens)
})

t.Run("filter SecretID for acl:read", func(t *testing.T) {
rules := `
acl = "read"
`
readOnlyToken, err := upsertTestTokenWithPolicyRules(codec, TestDefaultMasterToken, "dc1", rules)
require.NoError(t, err)

req := structs.ACLTokenListRequest{
Datacenter: "dc1",
QueryOptions: structs.QueryOptions{Token: readOnlyToken.SecretID},
}

resp := structs.ACLTokenListResponse{}

err = acl.TokenList(&req, &resp)
require.NoError(t, err)

tokens := []string{
masterTokenAccessorID,
structs.ACLTokenAnonymousID,
readOnlyToken.AccessorID,
t1.AccessorID,
t2.AccessorID,
}
require.ElementsMatch(t, gatherIDs(t, resp.Tokens), tokens)
for _, token := range resp.Tokens {
require.Equal(t, redactedToken, token.SecretID)
}
})
}

func TestACLEndpoint_TokenBatchRead(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions agent/structs/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ type ACLTokens []*ACLToken

type ACLTokenListStub struct {
AccessorID string
SecretID string
Description string
Policies []ACLTokenPolicyLink `json:",omitempty"`
Roles []ACLTokenRoleLink `json:",omitempty"`
Expand All @@ -578,6 +579,7 @@ type ACLTokenListStubs []*ACLTokenListStub
func (token *ACLToken) Stub() *ACLTokenListStub {
return &ACLTokenListStub{
AccessorID: token.AccessorID,
SecretID: token.SecretID,
Description: token.Description,
Policies: token.Policies,
Roles: token.Roles,
Expand Down
2 changes: 2 additions & 0 deletions agent/structs/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ func TestStructs_ACLToken_Stub(t *testing.T) {
stub := token.Stub()

require.Equal(t, token.AccessorID, stub.AccessorID)
require.Equal(t, token.SecretID, stub.SecretID)
require.Equal(t, token.Description, stub.Description)
require.Equal(t, token.Policies, stub.Policies)
require.Equal(t, token.Local, stub.Local)
Expand All @@ -286,6 +287,7 @@ func TestStructs_ACLToken_Stub(t *testing.T) {

stub := token.Stub()
require.Equal(t, token.AccessorID, stub.AccessorID)
require.Equal(t, token.SecretID, stub.SecretID)
require.Equal(t, token.Description, stub.Description)
require.Equal(t, token.Policies, stub.Policies)
require.Equal(t, token.Local, stub.Local)
Expand Down
1 change: 1 addition & 0 deletions api/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type ACLTokenListEntry struct {
CreateIndex uint64
ModifyIndex uint64
AccessorID string
SecretID string
Description string
Policies []*ACLTokenPolicyLink `json:",omitempty"`
Roles []*ACLTokenRoleLink `json:",omitempty"`
Expand Down
3 changes: 3 additions & 0 deletions api/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ func TestAPI_ACLToken_List(t *testing.T) {
token1, ok := tokenMap[created1.AccessorID]
require.True(t, ok)
require.NotNil(t, token1)
require.Equal(t, created1.SecretID, token1.SecretID)
require.Equal(t, created1.Description, token1.Description)
require.Equal(t, created1.CreateIndex, token1.CreateIndex)
require.Equal(t, created1.ModifyIndex, token1.ModifyIndex)
Expand All @@ -604,6 +605,7 @@ func TestAPI_ACLToken_List(t *testing.T) {
token2, ok := tokenMap[created2.AccessorID]
require.True(t, ok)
require.NotNil(t, token2)
require.Equal(t, created2.SecretID, token2.SecretID)
require.Equal(t, created2.Description, token2.Description)
require.Equal(t, created2.CreateIndex, token2.CreateIndex)
require.Equal(t, created2.ModifyIndex, token2.ModifyIndex)
Expand All @@ -613,6 +615,7 @@ func TestAPI_ACLToken_List(t *testing.T) {
token3, ok := tokenMap[created3.AccessorID]
require.True(t, ok)
require.NotNil(t, token3)
require.Equal(t, created3.SecretID, token3.SecretID)
require.Equal(t, created3.Description, token3.Description)
require.Equal(t, created3.CreateIndex, token3.CreateIndex)
require.Equal(t, created3.ModifyIndex, token3.ModifyIndex)
Expand Down
1 change: 1 addition & 0 deletions command/acl/token/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func (f *prettyFormatter) formatTokenListEntry(token *api.ACLTokenListEntry) str
var buffer bytes.Buffer

buffer.WriteString(fmt.Sprintf("AccessorID: %s\n", token.AccessorID))
buffer.WriteString(fmt.Sprintf("SecretID: %s\n", token.SecretID))
if token.Namespace != "" {
buffer.WriteString(fmt.Sprintf("Namespace: %s\n", token.Namespace))
}
Expand Down
3 changes: 3 additions & 0 deletions command/acl/token/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func TestFormatTokenList(t *testing.T) {
tokens: []*api.ACLTokenListEntry{
{
AccessorID: "fbd2447f-7479-4329-ad13-b021d74f86ba",
SecretID: "257ade69-748c-4022-bafd-76d27d9143f8",
mkeeler marked this conversation as resolved.
Show resolved Hide resolved
Description: "test token",
Local: false,
CreateTime: time.Date(2020, 5, 22, 18, 52, 31, 0, time.UTC),
Expand All @@ -168,6 +169,7 @@ func TestFormatTokenList(t *testing.T) {
tokens: []*api.ACLTokenListEntry{
{
AccessorID: "8acc7486-ca54-4d3c-9aed-5cd85651b0ee",
SecretID: "257ade69-748c-4022-bafd-76d27d9143f8",
Description: "legacy",
Legacy: true,
},
Expand All @@ -177,6 +179,7 @@ func TestFormatTokenList(t *testing.T) {
tokens: []*api.ACLTokenListEntry{
{
AccessorID: "fbd2447f-7479-4329-ad13-b021d74f86ba",
SecretID: "257ade69-748c-4022-bafd-76d27d9143f8",
Namespace: "foo",
Description: "test token",
Local: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"CreateIndex": 42,
"ModifyIndex": 100,
"AccessorID": "fbd2447f-7479-4329-ad13-b021d74f86ba",
"SecretID": "257ade69-748c-4022-bafd-76d27d9143f8",
"Description": "test token",
"Local": false,
"CreateTime": "2020-05-22T18:52:31Z",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Description: test token
Local: false
Create Time: 2020-05-22 18:52:31 +0000 UTC
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Description: test token
Local: false
Create Time: 2020-05-22 18:52:31 +0000 UTC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"CreateIndex": 5,
"ModifyIndex": 10,
"AccessorID": "fbd2447f-7479-4329-ad13-b021d74f86ba",
"SecretID": "257ade69-748c-4022-bafd-76d27d9143f8",
"Description": "test token",
"Policies": [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Namespace: foo
Description: test token
Local: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: fbd2447f-7479-4329-ad13-b021d74f86ba
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Namespace: foo
Description: test token
Local: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"CreateIndex": 0,
"ModifyIndex": 0,
"AccessorID": "8acc7486-ca54-4d3c-9aed-5cd85651b0ee",
"SecretID": "257ade69-748c-4022-bafd-76d27d9143f8",
"Description": "legacy",
"Local": false,
"CreateTime": "0001-01-01T00:00:00Z",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: 8acc7486-ca54-4d3c-9aed-5cd85651b0ee
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Description: legacy
Local: false
Create Time: 0001-01-01 00:00:00 +0000 UTC
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
AccessorID: 8acc7486-ca54-4d3c-9aed-5cd85651b0ee
SecretID: 257ade69-748c-4022-bafd-76d27d9143f8
Description: legacy
Local: false
Create Time: 0001-01-01 00:00:00 +0000 UTC
Expand Down
10 changes: 8 additions & 2 deletions website/content/api-docs/acl/tokens.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -594,13 +594,17 @@ $ curl -X GET http://127.0.0.1:8500/v1/acl/tokens

### Sample Response

-> **Note** - The token secret IDs are not included in the listing and must be
retrieved by the [token reading endpoint](#read-a-token)
-> **Note** If the token used for accessing the API has `acl:write` permissions,
then the `SecretID` will contain the tokens real value. Only when accessed with
a token with only `acl:read` permissions will the `SecretID` be redacted. This
is to prevent privilege escalation whereby having `acl:read` privileges allows
for reading other secrets which given even more permissions.

```json
[
{
"AccessorID": "6a1253d2-1785-24fd-91c2-f8e78c745511",
"SecretID": "<hidden>",
"Description": "Agent token for 'my-agent'",
"Policies": [
{
Expand All @@ -620,6 +624,7 @@ retrieved by the [token reading endpoint](#read-a-token)
},
{
"AccessorID": "00000000-0000-0000-0000-000000000002",
"SecretID": "<hidden>",
"Description": "Anonymous Token",
"Policies": null,
"Local": false,
Expand All @@ -630,6 +635,7 @@ retrieved by the [token reading endpoint](#read-a-token)
},
{
"AccessorID": "3328f9a6-433c-02d0-6649-7d07268dfec7",
"SecretID": "<hidden>",
"Description": "Bootstrap Token (Global Management)",
"Policies": [
{
Expand Down