Skip to content

Commit

Permalink
sql: Node user always passes CheckAnyPrivilege
Browse files Browse the repository at this point in the history
No release note since this only affects what is shown
in InternalExecutor queries.

Release note: None
  • Loading branch information
RichardJCai committed Mar 30, 2022
1 parent 7f8bdf2 commit db476bd
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ go_test(
"alter_column_type_test.go",
"ambiguous_commit_test.go",
"as_of_test.go",
"authorization_test.go",
"backfill_num_ranges_in_span_test.go",
"backfill_test.go",
"builtin_mem_usage_test.go",
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,12 @@ func (p *planner) CheckAnyPrivilege(ctx context.Context, descriptor catalog.Desc
}

user := p.SessionData().User()

if user.IsNodeUser() {
// User "node" has all privileges.
return nil
}

privs := descriptor.GetPrivileges()

// Check if 'user' itself has privileges.
Expand Down
82 changes: 82 additions & 0 deletions pkg/sql/authorization_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package sql_test

import (
"context"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/stretchr/testify/require"
)

func TestCheckAnyPrivilegeForNodeUser(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()
s, _, kv := serverutils.StartServer(t, base.TestServerArgs{})

defer s.Stopper().Stop(ctx)

ts := s.(*server.TestServer)

require.NotNil(t, ts.InternalExecutor())

ie := ts.InternalExecutor().(sqlutil.InternalExecutor)

txn := kv.NewTxn(ctx, "get-all-databases")
row, err := ie.QueryRowEx(
ctx, "get-all-databases", txn, sessiondata.NodeUserSessionDataOverride,
"SELECT count(1) FROM crdb_internal.databases",
)
require.NoError(t, err)
// 3 databases (system, defaultdb, postgres).
require.Equal(t, row.String(), "(3)")

_, err = ie.ExecEx(ctx, "create-database1", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"CREATE DATABASE test1")
require.NoError(t, err)

_, err = ie.ExecEx(ctx, "create-database2", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"CREATE DATABASE test2")
require.NoError(t, err)

// Revoke CONNECT on all non-system databases and ensure that when querying
// with node, we can still see all the databases.
_, err = ie.ExecEx(ctx, "revoke-privileges", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"REVOKE CONNECT ON DATABASE test1 FROM public")
require.NoError(t, err)
_, err = ie.ExecEx(ctx, "revoke-privileges", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"REVOKE CONNECT ON DATABASE test2 FROM public")
require.NoError(t, err)
_, err = ie.ExecEx(ctx, "revoke-privileges", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"REVOKE CONNECT ON DATABASE defaultdb FROM public")
require.NoError(t, err)
_, err = ie.ExecEx(ctx, "revoke-privileges", txn, sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"REVOKE CONNECT ON DATABASE postgres FROM public")
require.NoError(t, err)

row, err = ie.QueryRowEx(
ctx, "get-all-databases", txn, sessiondata.NodeUserSessionDataOverride,
"SELECT count(1) FROM crdb_internal.databases",
)
require.NoError(t, err)
// 3 databases (system, defaultdb, postgres, test1, test2).
require.Equal(t, row.String(), "(5)")
}

0 comments on commit db476bd

Please sign in to comment.