Adds support for agent-side ACL token management via API instead of config files. #3324
Conversation
slackpad
force-pushed
the
acl-intro-agent
branch
from
9d6ff91 to
c2f4670
Compare
slackpad
force-pushed
the
acl-intro-agent
branch
from
c2f4670 to
9b53738
Compare
slackpad
added
type/enhancement
agent/token_store.go
Outdated
| } | ||
|
|
||
| // UpdateUserToken replaces the current user token in the store. | ||
| func (t *TokenStore) UpdateUserToken(userToken string) { |
agent/token_store.go
Outdated
| } | ||
|
|
||
| // UpdateAgentToken replaces the current agent token in the store. | ||
| func (t *TokenStore) UpdateAgentToken(agentToken string) { |
agent/token_store.go
Outdated
| } | ||
|
|
||
| // GetTokenForUser returns the best token to use for user operations. | ||
| func (t *TokenStore) GetTokenForUser() string { |
There was a problem hiding this comment.
s/GetTokenForUser/UserToken/
agent/token_store.go
Outdated
| t.l.RLock() | ||
| defer t.l.RUnlock() | ||
|
|
||
| if t.userToken != "" { |
There was a problem hiding this comment.
That's redundant. return t.userToken should be sufficient.
agent/token_store.go
Outdated
| } | ||
|
|
||
| // GetTokenForAgent returns the best token to use for internal agent operations. | ||
| func (t *TokenStore) GetTokenForAgent() string { |
There was a problem hiding this comment.
s/GetTokenForAgent/AgentToken/
agent/token_store_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func TestTokenStore_UserToken(t *testing.T) { |
There was a problem hiding this comment.
If you rewrite the previous test as a table driven test then you can roll this test in there as well.
agent/token_store_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func TestTokenStore_AgentMasterToken(t *testing.T) { |
There was a problem hiding this comment.
This test feels a bit verbose. Not sure if a helper func would make it shorter but less readable though. You could add a helper to bail on wrong tokens but I'm not sure it really helps.
isNot := func(toks ...string) {
for _, tok := range toks {
if tokens.IsAgentMasterToken(tok) {
t.Fatalf("token %q is agent master token although it shouldn't be", tok)
}
}
}
...
isNot("", "nope")
agent/agent_endpoint_test.go
Outdated
| a := NewTestAgent(t.Name(), TestACLConfig()) | ||
| defer a.Shutdown() | ||
|
|
||
| t.Run("bad method", func(t *testing.T) { |
There was a problem hiding this comment.
test bad json as well? Also, you can write this as a table driven test
tests := []struct {
name string
method, urlstr string
body io.Reader
code int
agentToken, masterToken string
}{
{"bad method", "HEAD", "...", nil, http.StatusMethodNotAllowed, "", "" },
...
}
agent/agent_endpoint_test.go
Outdated
| }) | ||
| } | ||
|
|
||
| func TestAgent_Token_ACLDeny(t *testing.T) { |
There was a problem hiding this comment.
Roll this one into the previous test if you refactor to table driven?
agent/agent_endpoint_test.go
Outdated
| } | ||
| }) | ||
|
|
||
| t.Run("management token", func(t *testing.T) { |
There was a problem hiding this comment.
This works b/c the mgmt token is root set in TestACLConfig? Just curious?
There was a problem hiding this comment.
Yes - "root" is the default management token in all our test helpers. That's a little obscure.
| @@ -155,7 +149,7 @@ func (m *aclManager) lookupACL(a *Agent, id string) (acl.ACL, error) { | |||
| id = anonymousToken | |||
| } else if acl.RootACL(id) != nil { | |||
| return nil, errors.New(rootDenied) | |||
| } else if m.master != nil && id == a.config.ACLAgentMasterToken { | |||
| } else if a.tokens.IsAgentMasterToken(id) { | |||
There was a problem hiding this comment.
can agent.tokens be nil?
There was a problem hiding this comment.
No - the token store is created as part of constructing an agent, so it will be there even if ACLs aren't enabled.
This is the first phase of secure ACL token introduction where we change the agent-side token management to happen via a new token store instead of the config object directly, and then we add an API to manage the token store on the fly. This lets us use APIs to manage the agent tokens vs. having to place ACL tokens in config files on the agent.
When we do the server side we will cover the other tokens:
acl_replication_tokenandacl_master_token. Theacl_replication_tokenwill get added to the token store (we don't expect to add any other things to the token store), and theacl_master_tokenwill be handled with a new /v1/acl/bootstrap API.