From 4aea25ca80a7d15eb8d5115c1a3487e7960b37f4 Mon Sep 17 00:00:00 2001 From: Shengzhe Yao Date: Mon, 27 Jul 2015 10:25:12 -0700 Subject: [PATCH] fix queryservice integration tests related to tableacl 1. Deny any table access by default if tableacl does not find a ACL entry. 2. Fix queryservice integration test config file. 3. Log tableacl error only when 'queryserver-config-strict-table-acl' flag is turned on (default: off) --- go/vt/tableacl/tableacl.go | 2 +- go/vt/tableacl/tableacl_test.go | 8 ++--- go/vt/tabletserver/query_executor.go | 4 +-- test/test_data/table_acl_config.json | 49 ++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/go/vt/tableacl/tableacl.go b/go/vt/tableacl/tableacl.go index 5d6b7150074..d8a9df58c99 100644 --- a/go/vt/tableacl/tableacl.go +++ b/go/vt/tableacl/tableacl.go @@ -197,7 +197,7 @@ func Authorized(table string, role Role) acl.ACL { start = mid + 1 } } - return acl.AcceptAllACL{} + return acl.DenyAllACL{} } // GetCurrentConfig returns a copy of current tableacl configuration. diff --git a/go/vt/tableacl/tableacl_test.go b/go/vt/tableacl/tableacl_test.go index 7e240f9560f..f8ad6ddf751 100644 --- a/go/vt/tableacl/tableacl_test.go +++ b/go/vt/tableacl/tableacl_test.go @@ -48,8 +48,8 @@ func TestInitWithValidConfig(t *testing.T) { func TestInitFromProto(t *testing.T) { setUpTableACL(&simpleacl.Factory{}) readerACL := Authorized("my_test_table", READER) - if !reflect.DeepEqual(readerACL, acl.AcceptAllACL{}) { - t.Fatalf("tableacl has not been initialized, got: %v, want: %v", readerACL, acl.AcceptAllACL{}) + if !reflect.DeepEqual(readerACL, acl.DenyAllACL{}) { + t.Fatalf("tableacl has not been initialized, got: %v, want: %v", readerACL, acl.DenyAllACL{}) } config := &tableaclpb.Config{ TableGroups: []*tableaclpb.TableGroupSpec{{ @@ -67,8 +67,8 @@ func TestInitFromProto(t *testing.T) { } readerACL = Authorized("unknown_table", READER) - if !reflect.DeepEqual(acl.AcceptAllACL{}, readerACL) { - t.Fatalf("there is no config for unknown_table, should grand all permissions") + if !reflect.DeepEqual(acl.DenyAllACL{}, readerACL) { + t.Fatalf("there is no config for unknown_table, should deny by default") } readerACL = Authorized("test_table", READER) diff --git a/go/vt/tabletserver/query_executor.go b/go/vt/tabletserver/query_executor.go index d1e69c7b968..f7153c4f5ed 100644 --- a/go/vt/tabletserver/query_executor.go +++ b/go/vt/tabletserver/query_executor.go @@ -230,13 +230,13 @@ func (qre *QueryExecutor) checkPermissions() error { qre.qe.tableaclPseudoDenied.Add(tableACLStatsKey, 1) return nil } - errStr := fmt.Sprintf("table acl error: %q cannot run %v on table %q", username, qre.plan.PlanId, qre.plan.TableName) // raise error if in strictTableAcl mode, else just log an error. if qre.qe.strictTableAcl { + errStr := fmt.Sprintf("table acl error: %q cannot run %v on table %q", username, qre.plan.PlanId, qre.plan.TableName) qre.qe.tableaclDenied.Add(tableACLStatsKey, 1) + qre.qe.accessCheckerLogger.Errorf("%s", errStr) return NewTabletError(ErrFail, "%s", errStr) } - qre.qe.accessCheckerLogger.Errorf("%s", errStr) return nil } qre.qe.tableaclAllowed.Add(tableACLStatsKey, 1) diff --git a/test/test_data/table_acl_config.json b/test/test_data/table_acl_config.json index 0950cf82b57..9c5d1cbd547 100644 --- a/test/test_data/table_acl_config.json +++ b/test/test_data/table_acl_config.json @@ -1,5 +1,54 @@ { "table_groups": [ + { + "name": "mysql", + "table_names_or_prefixes": [""], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc_cached", + "table_names_or_prefixes": ["vtocc_nocache", "vtocc_cached%"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc_renamed", + "table_names_or_prefixes": ["vtocc_renamed%"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc_part", + "table_names_or_prefixes": ["vtocc_part%"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc", + "table_names_or_prefixes": ["vtocc_a", "vtocc_b", "vtocc_c", "dual", "vtocc_d", "vtocc_temp", "vtocc_e", "vtocc_f", "vtocc_strings", "vtocc_fracts", "vtocc_ints", "vtocc_misc", "vtocc_big", "vtocc_view"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc_test", + "table_names_or_prefixes": ["vtocc_test"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, + { + "name": "vtocc_acl_unmatched", + "table_names_or_prefixes": ["vtocc_acl_unmatched"], + "readers": ["youtube-dev-dedicated"], + "writers": ["youtube-dev-dedicated"], + "admins": ["youtube-dev-dedicated"] + }, { "name": "vtocc_acl_no_access", "table_names_or_prefixes": ["vtocc_acl_no_access"]