From 53699aa24fa5913e9bd20d325a0e7edd275cf181 Mon Sep 17 00:00:00 2001 From: Mark Rushakoff Date: Thu, 9 Feb 2017 11:32:52 -0800 Subject: [PATCH] Allow non-admin users to execute SHOW DATABASES This commit introduces a new interface type, influxql.Authorizer, that is passed as part of a statement's execution context and determines whether the context is permitted to access a given database. In the future, the Authorizer interface may be expanded to other resources besides databases. In this commit, the Authorizer interface is specifically used to determine which databases are returned when executing SHOW DATABASES. When HTTP authentication is enabled, the existing meta.UserInfo struct implements Authorizer, meaning admin users can SHOW every database, and non-admin users can SHOW only databases for which they have read and/or write permission. When HTTP authentication is disabled, all databases are visible through SHOW DATABASES. This addresses a long-standing issue where Chronograf or Grafana would be unable to list databases if the logged-in user did not have admin privileges. Fixes #4785. --- CHANGELOG.md | 7 +- cmd/influxd/run/server_test.go | 105 +++++++++++++++++++++++++ coordinator/statement_executor.go | 10 ++- coordinator/statement_executor_test.go | 51 ++++++++++++ influxql/ast.go | 5 +- influxql/ast_test.go | 73 +++++++++++++++++ influxql/query_executor.go | 19 +++++ services/httpd/handler.go | 8 ++ services/meta/data.go | 21 +++-- services/meta/data_test.go | 15 ++++ services/meta/query_authorizer.go | 2 +- services/meta/write_authorizer.go | 2 +- 12 files changed, 303 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 386ce381cf..f85b16088b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ ### Features -- [#7776](https://github.com/influxdata/influxdb/issues/7776): Add system information to /debug/vars. -- [#7948](https://github.com/influxdata/influxdb/pull/7948): Reduce memory allocations by reusing gzip.Writers across requests -- [#7553](https://github.com/influxdata/influxdb/issues/7553): Add modulo operator to the query language. - [#7977](https://github.com/influxdata/influxdb/issues/7977): Add chunked request processing back into the Go client v2 +- [#7974](https://github.com/influxdata/influxdb/pull/7974): Allow non-admin users to execute SHOW DATABASES. +- [#7948](https://github.com/influxdata/influxdb/pull/7948): Reduce memory allocations by reusing gzip.Writers across requests +- [#7776](https://github.com/influxdata/influxdb/issues/7776): Add system information to /debug/vars. +- [#7553](https://github.com/influxdata/influxdb/issues/7553): Add modulo operator to the query language. ## v1.2.1 [unreleased] diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index 4987248bcc..abff96f0af 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -293,6 +293,111 @@ func TestServer_DatabaseRetentionPolicyAutoCreate(t *testing.T) { } } +func TestServer_ShowDatabases_NoAuth(t *testing.T) { + t.Parallel() + s := OpenServer(NewConfig()) + defer s.Close() + + test := Test{ + queries: []*Query{ + &Query{ + name: "create db1", + command: "CREATE DATABASE db1", + exp: `{"results":[{"statement_id":0}]}`, + }, + &Query{ + name: "create db2", + command: "CREATE DATABASE db2", + exp: `{"results":[{"statement_id":0}]}`, + }, + &Query{ + name: "show dbs", + command: "SHOW DATABASES", + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"],"values":[["db1"],["db2"]]}]}]}`, + }, + }, + } + + for _, query := range test.queries { + if query.skip { + t.Logf("SKIP:: %s", query.name) + continue + } + if err := query.Execute(s); err != nil { + t.Error(fmt.Sprintf("command: %s - err: %s", query.command, query.Error(err))) + } else if !query.success() { + t.Error(query.failureMessage()) + } + } +} + +func TestServer_ShowDatabases_WithAuth(t *testing.T) { + t.Parallel() + c := NewConfig() + c.HTTPD.AuthEnabled = true + s := OpenServer(c) + defer s.Close() + + adminParams := map[string][]string{"u": []string{"admin"}, "p": []string{"admin"}} + readerParams := map[string][]string{"u": []string{"reader"}, "p": []string{"r"}} + writerParams := map[string][]string{"u": []string{"writer"}, "p": []string{"w"}} + nobodyParams := map[string][]string{"u": []string{"nobody"}, "p": []string{"n"}} + + test := Test{ + queries: []*Query{ + &Query{ + name: "create admin", + command: `CREATE USER admin WITH PASSWORD 'admin' WITH ALL PRIVILEGES`, + exp: `{"results":[{"statement_id":0}]}`, + }, + &Query{ + name: "create databases", + command: "CREATE DATABASE dbR; CREATE DATABASE dbW", + params: adminParams, + exp: `{"results":[{"statement_id":0},{"statement_id":1}]}`, + }, + &Query{ + name: "show dbs as admin", + command: "SHOW DATABASES", + params: adminParams, + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"],"values":[["dbR"],["dbW"]]}]}]}`, + }, + &Query{ + name: "create users", + command: `CREATE USER reader WITH PASSWORD 'r'; GRANT READ ON "dbR" TO "reader"; CREATE USER writer WITH PASSWORD 'w'; GRANT WRITE ON "dbW" TO "writer"; CREATE USER nobody WITH PASSWORD 'n'`, + params: adminParams, + exp: `{"results":[{"statement_id":0},{"statement_id":1},{"statement_id":2},{"statement_id":3},{"statement_id":4}]}`, + }, + &Query{ + name: "show dbs as reader", + command: "SHOW DATABASES", + params: readerParams, + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"],"values":[["dbR"]]}]}]}`, + }, + &Query{ + name: "show dbs as writer", + command: "SHOW DATABASES", + params: writerParams, + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"],"values":[["dbW"]]}]}]}`, + }, + &Query{ + name: "show dbs as nobody", + command: "SHOW DATABASES", + params: nobodyParams, + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"]}]}]}`, + }, + }, + } + + for _, query := range test.queries { + if err := query.Execute(s); err != nil { + t.Error(fmt.Sprintf("command: %s - err: %s", query.command, query.Error(err))) + } else if !query.success() { + t.Error(query.failureMessage()) + } + } +} + // Ensure user commands work. func TestServer_UserCommands(t *testing.T) { t.Parallel() diff --git a/coordinator/statement_executor.go b/coordinator/statement_executor.go index 1b84cd7eca..feea1a1ed7 100644 --- a/coordinator/statement_executor.go +++ b/coordinator/statement_executor.go @@ -156,7 +156,7 @@ func (e *StatementExecutor) ExecuteStatement(stmt influxql.Statement, ctx influx case *influxql.ShowContinuousQueriesStatement: rows, err = e.executeShowContinuousQueriesStatement(stmt) case *influxql.ShowDatabasesStatement: - rows, err = e.executeShowDatabasesStatement(stmt) + rows, err = e.executeShowDatabasesStatement(stmt, &ctx) case *influxql.ShowDiagnosticsStatement: rows, err = e.executeShowDiagnosticsStatement(stmt) case *influxql.ShowGrantsForUserStatement: @@ -620,12 +620,16 @@ func (e *StatementExecutor) executeShowContinuousQueriesStatement(stmt *influxql return rows, nil } -func (e *StatementExecutor) executeShowDatabasesStatement(q *influxql.ShowDatabasesStatement) (models.Rows, error) { +func (e *StatementExecutor) executeShowDatabasesStatement(q *influxql.ShowDatabasesStatement, ctx *influxql.ExecutionContext) (models.Rows, error) { dis := e.MetaClient.Databases() + a := ctx.ExecutionOptions.Authorizer row := &models.Row{Name: "databases", Columns: []string{"name"}} for _, di := range dis { - row.Values = append(row.Values, []interface{}{di.Name}) + // Only include databases that the user is authorized to read or write. + if a.AuthorizeDatabase(influxql.ReadPrivilege, di.Name) || a.AuthorizeDatabase(influxql.WritePrivilege, di.Name) { + row.Values = append(row.Values, []interface{}{di.Name}) + } } return []*models.Row{row}, nil } diff --git a/coordinator/statement_executor_test.go b/coordinator/statement_executor_test.go index 401777a111..466b341a8b 100644 --- a/coordinator/statement_executor_test.go +++ b/coordinator/statement_executor_test.go @@ -193,6 +193,57 @@ func TestStatementExecutor_NormalizeDeleteSeries(t *testing.T) { } } +type mockAuthorizer struct { + AuthorizeDatabaseFn func(influxql.Privilege, string) bool +} + +func (a *mockAuthorizer) AuthorizeDatabase(p influxql.Privilege, name string) bool { + return a.AuthorizeDatabaseFn(p, name) +} + +func TestQueryExecutor_ExecuteQuery_ShowDatabases(t *testing.T) { + qe := influxql.NewQueryExecutor() + qe.StatementExecutor = &coordinator.StatementExecutor{ + MetaClient: &internal.MetaClientMock{ + DatabasesFn: func() []meta.DatabaseInfo { + return []meta.DatabaseInfo{ + {Name: "db1"}, {Name: "db2"}, {Name: "db3"}, {Name: "db4"}, + } + }, + }, + } + + opt := influxql.ExecutionOptions{ + Authorizer: &mockAuthorizer{ + AuthorizeDatabaseFn: func(p influxql.Privilege, name string) bool { + return name == "db2" || name == "db4" + }, + }, + } + + q, err := influxql.ParseQuery("SHOW DATABASES") + if err != nil { + t.Fatal(err) + } + + results := ReadAllResults(qe.ExecuteQuery(q, opt, make(chan struct{}))) + exp := []*influxql.Result{ + { + StatementID: 0, + Series: []*models.Row{{ + Name: "databases", + Columns: []string{"name"}, + Values: [][]interface{}{ + {"db2"}, {"db4"}, + }, + }}, + }, + } + if !reflect.DeepEqual(results, exp) { + t.Fatalf("unexpected results: exp %s, got %s", spew.Sdump(exp), spew.Sdump(results)) + } +} + // QueryExecutor is a test wrapper for coordinator.QueryExecutor. type QueryExecutor struct { *influxql.QueryExecutor diff --git a/influxql/ast.go b/influxql/ast.go index 16b5c7f5e2..dc3ab4f8fa 100644 --- a/influxql/ast.go +++ b/influxql/ast.go @@ -2673,7 +2673,10 @@ func (s *ShowDatabasesStatement) String() string { return "SHOW DATABASES" } // RequiredPrivileges returns the privilege required to execute a ShowDatabasesStatement. func (s *ShowDatabasesStatement) RequiredPrivileges() (ExecutionPrivileges, error) { - return ExecutionPrivileges{{Admin: true, Name: "", Privilege: AllPrivileges}}, nil + // SHOW DATABASES is one of few statements that have no required privileges. + // Anyone is allowed to execute it, but the returned results depend on the user's + // individual database permissions. + return ExecutionPrivileges{{Admin: false, Name: "", Privilege: NoPrivileges}}, nil } // CreateContinuousQueryStatement represents a command for creating a continuous query. diff --git a/influxql/ast_test.go b/influxql/ast_test.go index 07342270df..41033401cf 100644 --- a/influxql/ast_test.go +++ b/influxql/ast_test.go @@ -1465,6 +1465,79 @@ func TestSelect_SubqueryPrivileges(t *testing.T) { } } +func TestShow_Privileges(t *testing.T) { + for _, c := range []struct { + stmt influxql.Statement + exp influxql.ExecutionPrivileges + }{ + { + stmt: &influxql.ShowDatabasesStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.NoPrivileges}}, + }, + { + stmt: &influxql.ShowFieldKeysStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowMeasurementsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowQueriesStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowRetentionPoliciesStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowSeriesStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowShardGroupsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + { + stmt: &influxql.ShowShardsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + { + stmt: &influxql.ShowStatsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + { + stmt: &influxql.ShowSubscriptionsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + { + stmt: &influxql.ShowDiagnosticsStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + { + stmt: &influxql.ShowTagKeysStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowTagValuesStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: false, Privilege: influxql.ReadPrivilege}}, + }, + { + stmt: &influxql.ShowUsersStatement{}, + exp: influxql.ExecutionPrivileges{{Admin: true, Privilege: influxql.AllPrivileges}}, + }, + } { + got, err := c.stmt.RequiredPrivileges() + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(c.exp, got) { + t.Errorf("exp: %v, got: %v", c.exp, got) + } + } +} + func TestSources_Names(t *testing.T) { sources := influxql.Sources([]influxql.Source{ &influxql.Measurement{ diff --git a/influxql/query_executor.go b/influxql/query_executor.go index 65026a205a..60fe17c37d 100644 --- a/influxql/query_executor.go +++ b/influxql/query_executor.go @@ -59,11 +59,30 @@ func ErrMaxConcurrentQueriesLimitExceeded(n, limit int) error { return fmt.Errorf("max-concurrent-queries limit exceeded(%d, %d)", n, limit) } +// Authorizer reports whether certain operations are authorized. +type Authorizer interface { + // AuthorizeDatabase indicates whether the given Privilege is authorized on the database with the given name. + AuthorizeDatabase(p Privilege, name string) bool +} + +// OpenAuthorizer is the Authorizer used when authorization is disabled. +// It allows all operations. +type OpenAuthorizer struct{} + +var _ Authorizer = OpenAuthorizer{} + +// AuthorizeDatabase returns true to allow any operation on a database. +func (OpenAuthorizer) AuthorizeDatabase(Privilege, string) bool { return true } + // ExecutionOptions contains the options for executing a query. type ExecutionOptions struct { // The database the query is running against. Database string + // How to determine whether the query is allowed to execute, + // what resources can be returned in SHOW queries, etc. + Authorizer Authorizer + // The requested maximum number of points to return in each result. ChunkSize int diff --git a/services/httpd/handler.go b/services/httpd/handler.go index 76a6c61fe7..bc8b9fd6ea 100644 --- a/services/httpd/handler.go +++ b/services/httpd/handler.go @@ -390,6 +390,14 @@ func (h *Handler) serveQuery(w http.ResponseWriter, r *http.Request, user *meta. NodeID: nodeID, } + if h.Config.AuthEnabled { + // The current user determines the authorized actions. + opts.Authorizer = user + } else { + // Auth is disabled, so allow everything. + opts.Authorizer = influxql.OpenAuthorizer{} + } + // Make sure if the client disconnects we signal the query to abort var closing chan struct{} if !async { diff --git a/services/meta/data.go b/services/meta/data.go index 703290b9e5..4b8ce89cf5 100644 --- a/services/meta/data.go +++ b/services/meta/data.go @@ -1409,15 +1409,24 @@ func (cqi *ContinuousQueryInfo) unmarshal(pb *internal.ContinuousQueryInfo) { // UserInfo represents metadata about a user in the system. type UserInfo struct { - Name string - Hash string - Admin bool + // User's name. + Name string + + // Hashed password. + Hash string + + // Whether the user is an admin, i.e. allowed to do everything. + Admin bool + + // Map of database name to granted privilege. Privileges map[string]influxql.Privilege } -// Authorize returns true if the user is authorized and false if not. -func (ui *UserInfo) Authorize(privilege influxql.Privilege, database string) bool { - if ui.Admin { +var _ influxql.Authorizer = (*UserInfo)(nil) + +// AuthorizeDatabase returns true if the user is authorized for the given privilege on the given database. +func (ui *UserInfo) AuthorizeDatabase(privilege influxql.Privilege, database string) bool { + if ui.Admin || privilege == influxql.NoPrivileges { return true } p, ok := ui.Privileges[database] diff --git a/services/meta/data_test.go b/services/meta/data_test.go index 0e82320029..044970849f 100644 --- a/services/meta/data_test.go +++ b/services/meta/data_test.go @@ -109,3 +109,18 @@ func Test_Data_CreateRetentionPolicy(t *testing.T) { t.Fatal(err) } } + +func TestUserInfo_AuthorizeDatabase(t *testing.T) { + emptyUser := &meta.UserInfo{} + if !emptyUser.AuthorizeDatabase(influxql.NoPrivileges, "anydb") { + t.Fatal("expected NoPrivileges to be authorized but it wasn't") + } + if emptyUser.AuthorizeDatabase(influxql.ReadPrivilege, "anydb") { + t.Fatal("expected ReadPrivilege to prevent authorization, but it was authorized") + } + + adminUser := &meta.UserInfo{Admin: true} + if !adminUser.AuthorizeDatabase(influxql.AllPrivileges, "anydb") { + t.Fatalf("expected admin to be authorized but it wasn't") + } +} diff --git a/services/meta/query_authorizer.go b/services/meta/query_authorizer.go index 0815f51edb..f15e5bb520 100644 --- a/services/meta/query_authorizer.go +++ b/services/meta/query_authorizer.go @@ -82,7 +82,7 @@ func (a *QueryAuthorizer) AuthorizeQuery(u *UserInfo, query *influxql.Query, dat if db == "" { db = database } - if !u.Authorize(p.Privilege, db) { + if !u.AuthorizeDatabase(p.Privilege, db) { return &ErrAuthorize{ Query: query, User: u.Name, diff --git a/services/meta/write_authorizer.go b/services/meta/write_authorizer.go index 8d545db500..e8e8d6091c 100644 --- a/services/meta/write_authorizer.go +++ b/services/meta/write_authorizer.go @@ -19,7 +19,7 @@ func NewWriteAuthorizer(c *Client) *WriteAuthorizer { // AuthorizeWrite returns nil if the user has permission to write to the database. func (a WriteAuthorizer) AuthorizeWrite(username, database string) error { u, err := a.Client.User(username) - if err != nil || u == nil || !u.Authorize(influxql.WritePrivilege, database) { + if err != nil || u == nil || !u.AuthorizeDatabase(influxql.WritePrivilege, database) { return &ErrAuthorize{ Database: database, Message: fmt.Sprintf("%s not authorized to write to %s", username, database),