diff --git a/go/test/endtoend/encryption/encryptedtransport/encrypted_transport_test.go b/go/test/endtoend/encryption/encryptedtransport/encrypted_transport_test.go index c32d1605b43..75b50d3e7cc 100644 --- a/go/test/endtoend/encryption/encryptedtransport/encrypted_transport_test.go +++ b/go/test/endtoend/encryption/encryptedtransport/encrypted_transport_test.go @@ -189,8 +189,8 @@ func TestSecureTransport(t *testing.T) { require.NoError(t, err) err = vterrors.FromVTRPC(qr.Error) require.Error(t, err) - assert.Contains(t, err.Error(), "table acl error") - assert.Contains(t, err.Error(), "cannot run Select on table") + assert.Contains(t, err.Error(), "Select command denied to user") + assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)") // now restart vtgate in the mode where we don't use SSL // for client connections, but we copy effective caller id @@ -214,8 +214,8 @@ func TestSecureTransport(t *testing.T) { require.NoError(t, err) err = vterrors.FromVTRPC(qr.Error) require.Error(t, err) - assert.Contains(t, err.Error(), "table acl error") - assert.Contains(t, err.Error(), "cannot run Select on table") + assert.Contains(t, err.Error(), "Select command denied to user") + assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)") // 'vtgate client 1' is authorized to access vt_insert_test callerID := &vtrpc.CallerID{ @@ -236,8 +236,8 @@ func TestSecureTransport(t *testing.T) { require.NoError(t, err) err = vterrors.FromVTRPC(qr.Error) require.Error(t, err) - assert.Contains(t, err.Error(), "table acl error") - assert.Contains(t, err.Error(), "cannot run Select on table") + assert.Contains(t, err.Error(), "Select command denied to user") + assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)") clusterInstance.Teardown() } diff --git a/go/test/endtoend/mysqlserver/mysql_server_test.go b/go/test/endtoend/mysqlserver/mysql_server_test.go index 9fb6344778b..692b0e7da43 100644 --- a/go/test/endtoend/mysqlserver/mysql_server_test.go +++ b/go/test/endtoend/mysqlserver/mysql_server_test.go @@ -193,8 +193,8 @@ func TestSelectWithUnauthorizedUser(t *testing.T) { _, err = conn.ExecuteFetch("SELECT * from vt_insert_test limit 1", 1, false) require.NotNilf(t, err, "error expected, got nil") - assert.Contains(t, err.Error(), "table acl error") - assert.Contains(t, err.Error(), "cannot run Select on table") + assert.Contains(t, err.Error(), "Select command denied to user") + assert.Contains(t, err.Error(), "for table 'vt_insert_test' (ACL check error)") } func connectDB(t *testing.T, vtParams mysql.ConnParams, params ...string) *sql.DB { diff --git a/go/vt/vttablet/endtoend/acl_test.go b/go/vt/vttablet/endtoend/acl_test.go index f86229fea5f..0894c2838d0 100644 --- a/go/vt/vttablet/endtoend/acl_test.go +++ b/go/vt/vttablet/endtoend/acl_test.go @@ -19,9 +19,10 @@ package endtoend import ( "bytes" "encoding/json" - "strings" "testing" + "gotest.tools/assert" + "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/vttablet/endtoend/framework" "vitess.io/vitess/go/vt/vttablet/tabletserver/rules" @@ -32,7 +33,7 @@ import ( func TestTableACL(t *testing.T) { client := framework.NewClient() - aclErr := "table acl error" + aclErr := "command denied to user 'dev' for table" execCases := []struct { query string err string @@ -101,9 +102,7 @@ func TestTableACL(t *testing.T) { } continue } - if err == nil || !strings.HasPrefix(err.Error(), tcase.err) { - t.Errorf("Execute(%s): Error: %v, must start with %s", tcase.query, err, tcase.err) - } + assert.ErrorContains(t, err, tcase.err) } streamCases := []struct { @@ -131,9 +130,7 @@ func TestTableACL(t *testing.T) { } continue } - if err == nil || !strings.HasPrefix(err.Error(), tcase.err) { - t.Errorf("Error: %v, must start with %s", err, tcase.err) - } + assert.ErrorContains(t, err, tcase.err) } } diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index a2291c58d8f..38b2e30f312 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -444,7 +444,11 @@ func (qre *QueryExecutor) checkAccess(authorized *tableacl.ACLResult, tableName } if qre.tsv.qe.strictTableACL { - errStr := fmt.Sprintf("table acl error: %q %v cannot run %v on table %q", callerID.Username, callerID.Groups, qre.plan.PlanID, tableName) + groupStr := "" + if len(callerID.Groups) > 0 { + groupStr = fmt.Sprintf(", in groups [%s],", strings.Join(callerID.Groups, ", ")) + } + errStr := fmt.Sprintf("%s command denied to user '%s'%s for table '%s' (ACL check error)", qre.plan.PlanID.String(), callerID.Username, groupStr, tableName) qre.tsv.Stats().TableaclDenied.Add(statsKey, 1) qre.tsv.qe.accessCheckerLogger.Infof("%s", errStr) return vterrors.Errorf(vtrpcpb.Code_PERMISSION_DENIED, "%s", errStr) diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index 91f3a6f51a8..3a717b0f143 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -822,6 +822,7 @@ func TestQueryExecutorMessageStreamACL(t *testing.T) { callerID = &querypb.VTGateCallerID{ Username: "u2", + Groups: []string{"non-admin"}, } qre.ctx = callerid.NewContext(context.Background(), nil, callerID) // Should fail because u2 does not have permission. @@ -829,10 +830,7 @@ func TestQueryExecutorMessageStreamACL(t *testing.T) { return io.EOF }) - want := `table acl error: "u2" [] cannot run MessageStream on table "msg"` - if err == nil || err.Error() != want { - t.Errorf("qre.MessageStream(msg) error: %v, want %s", err, want) - } + assert.EqualError(t, err, `MessageStream command denied to user 'u2', in groups [non-admin], for table 'msg' (ACL check error)`) if code := vterrors.Code(err); code != vtrpcpb.Code_PERMISSION_DENIED { t.Fatalf("qre.Execute: %v, want %v", code, vtrpcpb.Code_PERMISSION_DENIED) } @@ -972,10 +970,8 @@ func TestQueryExecutorTableAclDualTableExempt(t *testing.T) { if code := vterrors.Code(err); code != vtrpcpb.Code_PERMISSION_DENIED { t.Fatalf("qre.Execute: %v, want %v", code, vtrpcpb.Code_PERMISSION_DENIED) } - wanterr := "table acl error" - if !strings.Contains(err.Error(), wanterr) { - t.Fatalf("qre.Execute: %v, want %s", err, wanterr) - } + + assert.EqualError(t, err, `SelectImpossible command denied to user 'basic_username' for table 'test_table' (ACL check error)`) // table acl should be ignored when querying against dual table query = "select @@version_comment from dual limit 1" @@ -1014,6 +1010,7 @@ func TestQueryExecutorTableAclExemptACL(t *testing.T) { username := "u2" callerID := &querypb.VTGateCallerID{ Username: username, + Groups: []string{"eng", "beta"}, } ctx := callerid.NewContext(context.Background(), nil, callerID) @@ -1039,10 +1036,7 @@ func TestQueryExecutorTableAclExemptACL(t *testing.T) { if code := vterrors.Code(err); code != vtrpcpb.Code_PERMISSION_DENIED { t.Fatalf("qre.Execute: %v, want %v", code, vtrpcpb.Code_PERMISSION_DENIED) } - wanterr := "table acl error" - if !strings.Contains(err.Error(), wanterr) { - t.Fatalf("qre.Execute: %v, want %s", err, wanterr) - } + assert.EqualError(t, err, `Select command denied to user 'u2', in groups [eng, beta], for table 'test_table' (ACL check error)`) // table acl should be ignored since this is an exempt user. username = "exempt-acl"