Skip to content

Commit

Permalink
Merge pull request #44194 from otan-cockroach/backport2.1-44167
Browse files Browse the repository at this point in the history 
release-2.1: server,sql: allow admin UI to view table details for non-admins
  • Loading branch information
@otan
otan committed Jan 22, 2020
2 parents c96086a + b64d5b0 commit 0f056a5
Show file tree
Hide file tree
Showing 10 changed files with 432 additions and 78 deletions.
2 changes: 2 additions & 0 deletions docs/generated/sql/functions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,8 @@ may increase either contention or retry errors, or both.</p>
</span></td></tr>
<tr><td><code>crdb_internal.force_retry(val: <a href="interval.html">interval</a>) &rarr; <a href="int.html">int</a></code></td><td><span class="funcdesc"><p>This function is used only by CockroachDB’s developers for testing purposes.</p>
</span></td></tr>
<tr><td><code>crdb_internal.get_namespace_id(parent_id: <a href="int.html">int</a>, name: <a href="string.html">string</a>) &rarr; <a href="int.html">int</a></code></td><td></td></tr>
<tr><td><code>crdb_internal.get_zone_config(namespace_id: <a href="int.html">int</a>) &rarr; <a href="bytes.html">bytes</a></code></td><td></td></tr>
<tr><td><code>crdb_internal.is_admin() &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>Retrieves the current user’s admin status.</p>
</span></td></tr>
<tr><td><code>crdb_internal.no_constant_folding(input: anyelement) &rarr; anyelement</code></td><td><span class="funcdesc"><p>This function is used only by CockroachDB’s developers for testing purposes.</p>
Expand Down
39 changes: 28 additions & 11 deletions pkg/server/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1995,20 +1995,31 @@ func (rs resultScanner) Scan(row tree.Datums, colName string, dst interface{}) e
func (s *adminServer) queryZone(
ctx context.Context, userName string, id sqlbase.ID,
) (config.ZoneConfig, bool, error) {
const query = `SELECT config FROM system.zones WHERE id = $1`
rows, _ /* cols */, err := s.server.internalExecutor.QueryWithUser(
ctx, "admin-query-zone", nil /* txn */, userName, query, id,
const query = `SELECT crdb_internal.get_zone_config($1)`
rows, cols, err := s.server.internalExecutor.QueryWithUser(
ctx,
"admin-query-zone",
nil, /* txn */
userName,
query,
id,
)
if err != nil {
return config.ZoneConfig{}, false, err
}

if len(rows) == 0 {
return config.ZoneConfig{}, false, nil
if len(rows) != 1 {
return config.ZoneConfig{}, false, errors.Errorf("invalid number of rows returned: %s (%d)", query, id)
}

var zoneBytes []byte
scanner := resultScanner{}
scanner := makeResultScanner(cols)
if isNull, err := scanner.IsNull(rows[0], cols[0].Name); err != nil {
return config.ZoneConfig{}, false, err
} else if isNull {
return config.ZoneConfig{}, false, nil
}

err = scanner.ScanIndex(rows[0], 0, &zoneBytes)
if err != nil {
return config.ZoneConfig{}, false, err
Expand Down Expand Up @@ -2041,20 +2052,26 @@ func (s *adminServer) queryZonePath(
func (s *adminServer) queryNamespaceID(
ctx context.Context, userName string, parentID sqlbase.ID, name string,
) (sqlbase.ID, error) {
const query = `SELECT id FROM system.namespace WHERE "parentID" = $1 AND name = $2`
rows, _ /* cols */, err := s.server.internalExecutor.QueryWithUser(
const query = `SELECT crdb_internal.get_namespace_id($1, $2)`
rows, cols, err := s.server.internalExecutor.QueryWithUser(
ctx, "admin-query-namespace-ID", nil /* txn */, userName, query, parentID, name,
)
if err != nil {
return 0, err
}

if len(rows) == 0 {
return 0, errors.Errorf("namespace %s with ParentID %d not found", name, parentID)
if len(rows) != 1 {
return 0, errors.Errorf("invalid number of rows returned: %s (%d, %s)", query, parentID, name)
}

var id int64
scanner := resultScanner{}
scanner := makeResultScanner(cols)
if isNull, err := scanner.IsNull(rows[0], cols[0].Name); err != nil {
return 0, err
} else if isNull {
return 0, errors.Errorf("namespace %s with ParentID %d not found", name, parentID)
}

err = scanner.ScanIndex(rows[0], 0, &id)
if err != nil {
return 0, err
Expand Down
158 changes: 95 additions & 63 deletions pkg/server/admin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ import (
func getAdminJSONProto(
ts serverutils.TestServerInterface, path string, response protoutil.Message,
) error {
return serverutils.GetJSONProto(ts, adminPrefix+path, response)
return getAdminJSONProtoWithAdminOption(ts, path, response, true)
}

func getAdminJSONProtoWithAdminOption(
ts serverutils.TestServerInterface, path string, response protoutil.Message, isAdmin bool,
) error {
return serverutils.GetJSONProtoWithAdminOption(ts, adminPrefix+path, response, isAdmin)
}

func postAdminJSONProto(
Expand Down Expand Up @@ -237,88 +243,114 @@ func TestAdminAPIDatabases(t *testing.T) {
ctx, span := ac.AnnotateCtxWithSpan(context.Background(), "test")
defer span.Finish()

// Test databases endpoint.
const testdb = "test"
query := "CREATE DATABASE " + testdb
if _, err := db.Exec(query); err != nil {
t.Fatal(err)
}

var resp serverpb.DatabasesResponse
if err := getAdminJSONProto(s, "databases", &resp); err != nil {
// We have to create the non-admin user before calling
// "GRANT ... TO authenticatedUserNameNoAdmin".
// This is done in "GetAuthenticatedHTTPClient".
if _, err := ts.GetAuthenticatedHTTPClient(false); err != nil {
t.Fatal(err)
}

expectedDBs := []string{"defaultdb", "postgres", "system", testdb}
if a, e := len(resp.Databases), len(expectedDBs); a != e {
t.Fatalf("length of result %d != expected %d", a, e)
}

sort.Strings(resp.Databases)
for i, e := range expectedDBs {
if a := resp.Databases[i]; a != e {
t.Fatalf("database name %s != expected %s", a, e)
}
}

// Test database details endpoint.
// Grant permissions to view the tables for the given viewing user.
privileges := []string{"SELECT", "UPDATE"}
testuser := "testuser"
createUserQuery := "CREATE USER " + testuser
if _, err := db.Exec(createUserQuery); err != nil {
query = fmt.Sprintf(
"GRANT %s ON DATABASE %s TO %s",
strings.Join(privileges, ", "),
testdb,
authenticatedUserNameNoAdmin,
)
if _, err := db.Exec(query); err != nil {
t.Fatal(err)
}

grantQuery := "GRANT " + strings.Join(privileges, ", ") + " ON DATABASE " + testdb + " TO " + testuser
if _, err := db.Exec(grantQuery); err != nil {
t.Fatal(err)
}
for _, tc := range []struct {
expectedDBs []string
isAdmin bool
}{
{[]string{"defaultdb", "postgres", "system", testdb}, true},
{[]string{testdb}, false},
} {
t.Run(fmt.Sprintf("isAdmin:%t", tc.isAdmin), func(t *testing.T) {
// Test databases endpoint.
var resp serverpb.DatabasesResponse
if err := getAdminJSONProtoWithAdminOption(
s,
"databases",
&resp,
tc.isAdmin,
); err != nil {
t.Fatal(err)
}

var details serverpb.DatabaseDetailsResponse
if err := getAdminJSONProto(s, "databases/"+testdb, &details); err != nil {
t.Fatal(err)
}
if a, e := len(resp.Databases), len(tc.expectedDBs); a != e {
t.Fatalf("length of result %d != expected %d", a, e)
}

if a, e := len(details.Grants), 4; a != e {
t.Fatalf("# of grants %d != expected %d", a, e)
}
sort.Strings(resp.Databases)
for i, e := range tc.expectedDBs {
if a := resp.Databases[i]; a != e {
t.Fatalf("database name %s != expected %s", a, e)
}
}

userGrants := make(map[string][]string)
for _, grant := range details.Grants {
switch grant.User {
case sqlbase.AdminRole, security.RootUser, testuser:
userGrants[grant.User] = append(userGrants[grant.User], grant.Privileges...)
default:
t.Fatalf("unknown grant to user %s", grant.User)
}
}
for u, p := range userGrants {
switch u {
case sqlbase.AdminRole:
if !reflect.DeepEqual(p, []string{"ALL"}) {
t.Fatalf("privileges %v != expected %v", p, privileges)
// Test database details endpoint.
var details serverpb.DatabaseDetailsResponse
if err := getAdminJSONProtoWithAdminOption(
s,
"databases/"+testdb,
&details,
tc.isAdmin,
); err != nil {
t.Fatal(err)
}
case security.RootUser:
if !reflect.DeepEqual(p, []string{"ALL"}) {
t.Fatalf("privileges %v != expected %v", p, privileges)

if a, e := len(details.Grants), 4; a != e {
t.Fatalf("# of grants %d != expected %d", a, e)
}
case testuser:
sort.Strings(p)
if !reflect.DeepEqual(p, privileges) {
t.Fatalf("privileges %v != expected %v", p, privileges)

userGrants := make(map[string][]string)
for _, grant := range details.Grants {
switch grant.User {
case sqlbase.AdminRole, security.RootUser, authenticatedUserNameNoAdmin:
userGrants[grant.User] = append(userGrants[grant.User], grant.Privileges...)
default:
t.Fatalf("unknown grant to user %s", grant.User)
}
}
for u, p := range userGrants {
switch u {
case sqlbase.AdminRole:
if !reflect.DeepEqual(p, []string{"ALL"}) {
t.Fatalf("privileges %v != expected %v", p, privileges)
}
case security.RootUser:
if !reflect.DeepEqual(p, []string{"ALL"}) {
t.Fatalf("privileges %v != expected %v", p, privileges)
}
case authenticatedUserNameNoAdmin:
sort.Strings(p)
if !reflect.DeepEqual(p, privileges) {
t.Fatalf("privileges %v != expected %v", p, privileges)
}
default:
t.Fatalf("unknown grant to user %s", u)
}
}
default:
t.Fatalf("unknown grant to user %s", u)
}
}

// Verify Descriptor ID.
path, err := ts.admin.queryDescriptorIDPath(ctx, security.RootUser, []string{testdb})
if err != nil {
t.Fatal(err)
}
if a, e := details.DescriptorID, int64(path[1]); a != e {
t.Fatalf("db had descriptorID %d, expected %d", a, e)
// Verify Descriptor ID.
path, err := ts.admin.queryDescriptorIDPath(ctx, security.RootUser, []string{testdb})
if err != nil {
t.Fatal(err)
}
if a, e := details.DescriptorID, int64(path[1]); a != e {
t.Fatalf("db had descriptorID %d, expected %d", a, e)
}
})
}
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/sql/conn_executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1779,10 +1779,11 @@ func (ex *connExecutor) evalCtx(

return extendedEvalContext{
EvalContext: tree.EvalContext{
Planner: p,
Sequence: p,
SessionAccessor: p,
StmtTimestamp: stmtTS,
Planner: p,
Sequence: p,
SessionAccessor: p,
PrivilegedAccessor: p,
StmtTimestamp: stmtTS,

Txn: txn,
SessionData: &ex.sessionData,
Expand Down
29 changes: 29 additions & 0 deletions pkg/sql/descriptor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,35 @@ func getDescriptor(
return true, nil
}

// lookupDescriptorByID looks up the descriptor for `id` and returns it.
// It can be a table or database descriptor.
// Returns the descriptor (if found), a bool representing whether the
// descriptor was found and an error if any.
func lookupDescriptorByID(
ctx context.Context, txn *client.Txn, id sqlbase.ID,
) (sqlbase.DescriptorProto, bool, error) {
var desc sqlbase.DescriptorProto
for _, lookupFn := range []func() (sqlbase.DescriptorProto, error){
func() (sqlbase.DescriptorProto, error) {
return sqlbase.GetTableDescFromID(ctx, txn, id)
},
func() (sqlbase.DescriptorProto, error) {
return sqlbase.GetDatabaseDescFromID(ctx, txn, id)
},
} {
var err error
desc, err = lookupFn()
if err != nil {
if err == sqlbase.ErrDescriptorNotFound {
continue
}
return nil, false, err
}
return desc, true, nil
}
return nil, false, nil
}

// getDescriptorByID looks up the descriptor for `id`, validates it,
// and unmarshals it into `descriptor`.
//
Expand Down
Loading

0 comments on commit 0f056a5

Please sign in to comment.