luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

grpc/acl: fix bug where ACL token was required even if disabled (#15904) 

Fixes a bug introduced by #15346 where we'd always require an ACL
token even if ACLs were disabled because we were erroneously
treating `nil` identity as anonymous.
This commit is contained in:
Dan Upton 2023-01-05 16:31:18 +00:00 committed by GitHub
parent a36173e2b5
commit 76fea384a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
agent/grpc-external/services/connectca/sign_test.go
View File

@ -33,7 +33,7 @@ func TestSign_ConnectDisabled(t *testing.T) {
func TestSign_Validation(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
server := NewServer(Config{
Logger: hclog.NewNullLogger(),
@ -90,7 +90,7 @@ func TestSign_Unauthenticated(t *testing.T) {
func TestSign_PermissionDenied(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -116,7 +116,7 @@ func TestSign_PermissionDenied(t *testing.T) {
func TestSign_InvalidCSR(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -142,7 +142,7 @@ func TestSign_InvalidCSR(t *testing.T) {
func TestSign_RateLimited(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -168,7 +168,7 @@ func TestSign_RateLimited(t *testing.T) {
func TestSign_InternalError(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -194,7 +194,7 @@ func TestSign_InternalError(t *testing.T) {
func TestSign_Success(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -220,7 +220,7 @@ func TestSign_Success(t *testing.T) {
func TestSign_RPCForwarding(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAllowAll(t), nil)
Return(testutils.ACLsDisabled(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).

19
agent/grpc-external/services/dataplane/get_supported_features_test.go
View File

@ -53,6 +53,25 @@ func TestSupportedDataplaneFeatures_Success(t *testing.T) {
}
}
func TestSupportedDataplaneFeatures_ACLsDisabled(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", "", mock.Anything, mock.Anything).
Return(testutils.ACLsDisabled(t), nil)
options := structs.QueryOptions{Token: ""}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
require.NoError(t, err)
server := NewServer(Config{
Logger: hclog.NewNullLogger(),
ACLResolver: aclResolver,
})
client := testClient(t, server)
resp, err := client.GetSupportedDataplaneFeatures(ctx, &pbdataplane.GetSupportedDataplaneFeaturesRequest{})
require.NoError(t, err)
require.Equal(t, 3, len(resp.SupportedDataplaneFeatures))
}
func TestSupportedDataplaneFeatures_InvalidACLToken(t *testing.T) {
// Mock the ACL resolver to return ErrNotFound.
aclResolver := &MockACLResolver{}

5
agent/grpc-external/testutils/acl.go
View File

@ -23,12 +23,11 @@ func ACLAnonymous(t *testing.T) resolver.Result {
}
}
func ACLAllowAll(t *testing.T) resolver.Result {
func ACLsDisabled(t *testing.T) resolver.Result {
t.Helper()
return resolver.Result{
Authorizer: acl.AllowAll(),
ACLIdentity: randomACLIdentity(t),
Authorizer: acl.ManageAll(),
}
}

2
agent/grpc-external/utils.go
View File

@ -36,7 +36,7 @@ func RequireAnyValidACLToken(resolver ACLResolver, token string) error {
return status.Error(codes.Unauthenticated, err.Error())
}
if id := authz.ACLIdentity; id == nil || id.ID() == structs.ACLTokenAnonymousID {
if id := authz.ACLIdentity; id != nil && id.ID() == structs.ACLTokenAnonymousID {
return status.Error(codes.Unauthenticated, "An ACL token must be provided (via the `x-consul-token` metadata field) to call this endpoint")
}