From 1d20498273d2b65baf6d8247d0b3402ce590736c Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 22 Jun 2016 08:15:18 -0500 Subject: [PATCH] Allow a non-admin to call "use" for the influx cli Previously, a non-admin could not call "use" in the influx cli since the `SHOW DATABASES` command requires admin permissions to run. The correct solution to this is likely to allow non-admins to call `SHOW DATABASES`, but only see the databases they should be capable of seeing. Since we don't have this kind of fine-grained authorization yet and plans for it are still in the works, we do need someway to not arbitrarily cripple non-admins attempting to use the cli program. This is a temporary solution that will ignore any authorization errors from `SHOW DATABASES` if authorization has been set. A warning message will be printed and the database will be switched. This should be enough to ensure that there is some warning that you may not have switched to a valid database while not crippling non-admin users. A temporary solution for #6397. --- CHANGELOG.md | 1 + cmd/influx/cli/cli.go | 53 +++++++++++++++------------- cmd/influx/cli/cli_test.go | 71 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 99 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e78987076d5..6406dba27c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ With this release the systemd configuration files for InfluxDB will use the syst - [#6869](https://github.com/influxdata/influxdb/issues/6869): Remove FieldCodec from tsdb package. - [#6882](https://github.com/influxdata/influxdb/pull/6882): Remove a double lock in the tsm1 index writer. - [#6883](https://github.com/influxdata/influxdb/pull/6883): Rename dumptsmdev to dumptsm in influx_inspect. +- [#6864](https://github.com/influxdata/influxdb/pull/6864): Allow a non-admin to call "use" for the influx cli. ## v0.13.0 [2016-05-12] diff --git a/cmd/influx/cli/cli.go b/cmd/influx/cli/cli.go index a034ea95444..d4bf52bc0d2 100644 --- a/cmd/influx/cli/cli.go +++ b/cmd/influx/cli/cli.go @@ -337,41 +337,46 @@ func (c *CommandLine) use(cmd string) { } d := args[1] - // validate if specified database exists + // Validate if specified database exists response, err := c.Client.Query(client.Query{Command: "SHOW DATABASES"}) if err != nil { fmt.Printf("ERR: %s\n", err) return - } - - if err := response.Error(); err != nil { - fmt.Printf("ERR: %s\n", err) - return - } - - // verify the provided database exists - databaseExists := func() bool { - for _, result := range response.Results { - for _, row := range result.Series { - if row.Name == "databases" { - for _, values := range row.Values { - for _, database := range values { - if database == d { - return true + } else if err := response.Error(); err != nil { + if c.Username == "" { + fmt.Printf("ERR: %s\n", err) + return + } + // TODO(jsternberg): Fix SHOW DATABASES to be user-aware #6397. + // If we are unable to run SHOW DATABASES, display a warning and use the + // database anyway in case the person doesn't have permission to run the + // command, but does have permission to use the database. + fmt.Printf("WARN: %s\n", err) + } else { + // Verify the provided database exists + if databaseExists := func() bool { + for _, result := range response.Results { + for _, row := range result.Series { + if row.Name == "databases" { + for _, values := range row.Values { + for _, database := range values { + if database == d { + return true + } } } } } } + return false + }(); !databaseExists { + fmt.Printf("ERR: Database %s doesn't exist. Run SHOW DATABASES for a list of existing databases.\n", d) + return } - return false - }() - if databaseExists { - c.Database = d - fmt.Printf("Using database %s\n", d) - } else { - fmt.Printf("ERR: Database %s doesn't exist. Run SHOW DATABASES for a list of existing databases.\n", d) } + + c.Database = d + fmt.Printf("Using database %s\n", d) } // SetPrecision sets client precision diff --git a/cmd/influx/cli/cli_test.go b/cmd/influx/cli/cli_test.go index bb5f3934297..c2c7edae7dd 100644 --- a/cmd/influx/cli/cli_test.go +++ b/cmd/influx/cli/cli_test.go @@ -3,6 +3,7 @@ package cli_test import ( "bufio" "bytes" + "fmt" "io" "net" "net/http" @@ -272,7 +273,6 @@ func TestParseCommand_Use(t *testing.T) { if err != nil { t.Fatalf("unexpected error. expected %v, actual %v", nil, err) } - m := cli.CommandLine{Client: c} tests := []struct { cmd string @@ -286,6 +286,7 @@ func TestParseCommand_Use(t *testing.T) { } for _, test := range tests { + m := cli.CommandLine{Client: c} if err := m.ParseCommand(test.cmd); err != nil { t.Fatalf(`Got error %v for command %q, expected nil.`, err, test.cmd) } @@ -296,6 +297,59 @@ func TestParseCommand_Use(t *testing.T) { } } +func TestParseCommand_UseAuth(t *testing.T) { + t.Parallel() + ts := emptyTestServer() + defer ts.Close() + + u, _ := url.Parse(ts.URL) + tests := []struct { + cmd string + user string + database string + }{ + { + cmd: "use db", + user: "admin", + database: "db", + }, + { + cmd: "use blank", + user: "admin", + database: "", + }, + { + cmd: "use db", + user: "anonymous", + database: "db", + }, + { + cmd: "use blank", + user: "anonymous", + database: "blank", + }, + } + + for i, tt := range tests { + config := client.Config{URL: *u, Username: tt.user} + fmt.Println("using auth:", tt.user) + c, err := client.NewClient(config) + if err != nil { + t.Errorf("%d. unexpected error. expected %v, actual %v", i, nil, err) + continue + } + m := cli.CommandLine{Client: c, Username: tt.user} + + if err := m.ParseCommand(tt.cmd); err != nil { + t.Fatalf(`%d. Got error %v for command %q, expected nil.`, i, err, tt.cmd) + } + + if m.Database != tt.database { + t.Fatalf(`%d. Command "use" changed database to %q. Expected %q`, i, m.Database, tt.database) + } + } +} + func TestParseCommand_Consistency(t *testing.T) { t.Parallel() c := cli.CommandLine{} @@ -490,6 +544,14 @@ func emptyTestServer() *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-Influxdb-Version", SERVER_VERSION) + // Fake authorization entirely based on the username. + authorized := false + user, _, _ := r.BasicAuth() + switch user { + case "", "admin": + authorized = true + } + switch r.URL.Path { case "/query": values := r.URL.Query() @@ -503,7 +565,12 @@ func emptyTestServer() *httptest.Server { switch stmt.(type) { case *influxql.ShowDatabasesStatement: - io.WriteString(w, `{"results":[{"series":[{"name":"databases","columns":["name"],"values":[["db"]]}]}]}`) + if authorized { + io.WriteString(w, `{"results":[{"series":[{"name":"databases","columns":["name"],"values":[["db"]]}]}]}`) + } else { + w.WriteHeader(http.StatusUnauthorized) + io.WriteString(w, fmt.Sprintf(`{"error":"error authorizing query: %s not authorized to execute statement 'SHOW DATABASES', requires admin privilege"}`, user)) + } case *influxql.ShowDiagnosticsStatement: io.WriteString(w, `{"results":[{}]}`) }