diff --git a/server/sources.go b/server/sources.go index b87342c4af..40eb691481 100644 --- a/server/sources.go +++ b/server/sources.go @@ -76,7 +76,7 @@ func (s *Service) NewSource(w http.ResponseWriter, r *http.Request) { return } - if err := ValidSourceRequest(src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { + if err := ValidSourceRequest(&src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { invalidData(w, err, s.Logger) return } @@ -270,7 +270,7 @@ func (s *Service) UpdateSource(w http.ResponseWriter, r *http.Request) { return } - if err := ValidSourceRequest(src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { + if err := ValidSourceRequest(&src, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { invalidData(w, err, s.Logger) return } @@ -290,8 +290,11 @@ func (s *Service) UpdateSource(w http.ResponseWriter, r *http.Request) { encodeJSON(w, http.StatusOK, newSourceResponse(src), s.Logger) } -// ValidSourceRequest checks if name, url and type are valid -func ValidSourceRequest(s chronograf.Source, defaultOrgID string) error { +// ValidSourceRequest checks if name, url, type, and role are valid +func ValidSourceRequest(s *chronograf.Source, defaultOrgID string) error { + if s == nil { + return fmt.Errorf("source must be non-nil") + } // Name and URL areq required if s.URL == "" { return fmt.Errorf("url required") @@ -318,6 +321,14 @@ func ValidSourceRequest(s chronograf.Source, defaultOrgID string) error { if s.Role == "" { s.Role = roles.ViewerRoleName } + + switch s.Role { + case roles.ViewerRoleName, roles.EditorRoleName, roles.AdminRoleName: + return nil + default: + return fmt.Errorf("Unknown role %s. Valid roles are 'viewer', 'editor', and 'admin'", s.Role) + } + return nil } @@ -345,7 +356,7 @@ func (s *Service) HandleNewSources(ctx context.Context, input string) error { } for _, sk := range srcsKaps { - if err := ValidSourceRequest(sk.Source, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { + if err := ValidSourceRequest(&sk.Source, fmt.Sprintf("%d", defaultOrg.ID)); err != nil { return err } // Add any new sources and kapacitors as specified via server flag diff --git a/server/sources_test.go b/server/sources_test.go index eb717ef0a3..22f41a50c1 100644 --- a/server/sources_test.go +++ b/server/sources_test.go @@ -11,11 +11,216 @@ import ( "testing" "github.com/bouk/httprouter" + "github.com/google/go-cmp/cmp" "github.com/influxdata/chronograf" "github.com/influxdata/chronograf/log" "github.com/influxdata/chronograf/mocks" + "github.com/influxdata/chronograf/roles" ) +func Test_ValidSourceRequest(t *testing.T) { + type args struct { + source *chronograf.Source + defaultOrgID string + } + type wants struct { + err error + source *chronograf.Source + } + tests := []struct { + name string + args args + wants wants + }{ + { + name: "nil source", + args: args{}, + wants: wants{ + err: fmt.Errorf("source must be non-nil"), + }, + }, + { + name: "missing url", + args: args{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Telegraf: "telegraf", + Organization: "0", + Role: roles.ViewerRoleName, + }, + }, + wants: wants{ + err: fmt.Errorf("url required"), + }, + }, + { + name: "invalid source type", + args: args{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: "non-existent-type", + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Telegraf: "telegraf", + Organization: "0", + Role: roles.ViewerRoleName, + }, + }, + wants: wants{ + err: fmt.Errorf("invalid source type non-existent-type"), + }, + }, + { + name: "set organization to be default org if not specified", + args: args{ + defaultOrgID: "2", + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Telegraf: "telegraf", + Role: roles.ViewerRoleName, + }, + }, + wants: wants{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Organization: "2", + Telegraf: "telegraf", + Role: roles.ViewerRoleName, + }, + }, + }, + { + name: "bad url", + args: args{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "im a bad url", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Organization: "0", + Default: true, + Telegraf: "telegraf", + Role: roles.ViewerRoleName, + }, + }, + wants: wants{ + err: fmt.Errorf("invalid source URI: parse im a bad url: invalid URI for request"), + }, + }, + { + name: "set Role to be viewer if not specified", + args: args{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Telegraf: "telegraf", + Organization: "0", + }, + }, + wants: wants{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Organization: "0", + Telegraf: "telegraf", + Role: roles.ViewerRoleName, + }, + }, + }, + { + name: "bad role type", + args: args{ + source: &chronograf.Source{ + ID: 1, + Name: "I'm a really great source", + Type: chronograf.InfluxDB, + Username: "fancy", + Password: "i'm so", + SharedSecret: "supersecret", + URL: "http://www.any.url.com", + MetaURL: "http://www.so.meta.com", + InsecureSkipVerify: true, + Default: true, + Telegraf: "telegraf", + Organization: "0", + Role: "superperson", + }, + }, + wants: wants{ + err: fmt.Errorf("Unknown role superperson. Valid roles are 'viewer', 'editor', and 'admin'"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidSourceRequest(tt.args.source, tt.args.defaultOrgID) + if err == nil && tt.wants.err == nil { + if diff := cmp.Diff(tt.args.source, tt.wants.source); diff != "" { + t.Errorf("%q. ValidSourceRequest():\n-got/+want\ndiff %s", tt.name, diff) + } + return + } + if err.Error() != tt.wants.err.Error() { + t.Errorf("%q. ValidSourceRequest() = %q, want %q", tt.name, err, tt.wants.err) + } + }) + } +} + func Test_newSourceResponse(t *testing.T) { tests := []struct { name string diff --git a/server/stores_test.go b/server/stores_test.go index 6fdfa3cb61..602b1228b4 100644 --- a/server/stores_test.go +++ b/server/stores_test.go @@ -370,3 +370,296 @@ func TestStore_SourcesGet(t *testing.T) { }) } } + +func TestStore_SourcesAll(t *testing.T) { + type fields struct { + SourcesStore chronograf.SourcesStore + } + type args struct { + organization string + role string + } + type wants struct { + sources []chronograf.Source + err bool + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "Get viewer sources as viewer", + fields: fields{ + SourcesStore: &mocks.SourcesStore{ + AllF: func(ctx context.Context) ([]chronograf.Source, error) { + return []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + }, nil + }, + }, + }, + args: args{ + organization: "0", + role: "viewer", + }, + wants: wants{ + sources: []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + }, + }, + }, + { + name: "Get viewer sources as viewer - multiple orgs and multiple roles", + fields: fields{ + SourcesStore: &mocks.SourcesStore{ + AllF: func(ctx context.Context) ([]chronograf.Source, error) { + return []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 2, + Name: "A bad source", + Organization: "0", + Role: "editor", + }, + { + ID: 3, + Name: "A good source", + Organization: "0", + Role: "admin", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + { + ID: 5, + Name: "i'm in the wrong org", + Organization: "1", + Role: "viewer", + }, + }, nil + }, + }, + }, + args: args{ + organization: "0", + role: "viewer", + }, + wants: wants{ + sources: []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + }, + }, + }, + { + name: "Get editor sources as editor - multiple orgs and multiple roles", + fields: fields{ + SourcesStore: &mocks.SourcesStore{ + AllF: func(ctx context.Context) ([]chronograf.Source, error) { + return []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 2, + Name: "A bad source", + Organization: "0", + Role: "editor", + }, + { + ID: 3, + Name: "A good source", + Organization: "0", + Role: "admin", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + { + ID: 5, + Name: "i'm in the wrong org", + Organization: "1", + Role: "viewer", + }, + { + ID: 2, + Name: "i'm an editor, but wrong org", + Organization: "3", + Role: "editor", + }, + }, nil + }, + }, + }, + args: args{ + organization: "0", + role: "editor", + }, + wants: wants{ + sources: []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 2, + Name: "A bad source", + Organization: "0", + Role: "editor", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + }, + }, + }, + { + name: "Get admin sources as admin - multiple orgs and multiple roles", + fields: fields{ + SourcesStore: &mocks.SourcesStore{ + AllF: func(ctx context.Context) ([]chronograf.Source, error) { + return []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 2, + Name: "A bad source", + Organization: "0", + Role: "editor", + }, + { + ID: 3, + Name: "A good source", + Organization: "0", + Role: "admin", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + { + ID: 5, + Name: "i'm in the wrong org", + Organization: "1", + Role: "viewer", + }, + { + ID: 2, + Name: "i'm an editor, but wrong org", + Organization: "3", + Role: "editor", + }, + }, nil + }, + }, + }, + args: args{ + organization: "0", + role: "admin", + }, + wants: wants{ + sources: []chronograf.Source{ + { + ID: 1, + Name: "my sweet name", + Organization: "0", + Role: "viewer", + }, + { + ID: 2, + Name: "A bad source", + Organization: "0", + Role: "editor", + }, + { + ID: 3, + Name: "A good source", + Organization: "0", + Role: "admin", + }, + { + ID: 4, + Name: "a source I can has", + Organization: "0", + Role: "viewer", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := &Store{ + SourcesStore: tt.fields.SourcesStore, + } + + ctx := context.Background() + + if tt.args.organization != "" { + ctx = context.WithValue(ctx, organizations.ContextKey, tt.args.organization) + } + + if tt.args.role != "" { + ctx = context.WithValue(ctx, roles.ContextKey, tt.args.role) + } + + sources, err := store.Sources(ctx).All(ctx) + if (err != nil) != tt.wants.err { + t.Errorf("%q. Store.Sources().Get() error = %v, wantErr %v", tt.name, err, tt.wants.err) + return + } + if diff := cmp.Diff(sources, tt.wants.sources); diff != "" { + t.Errorf("%q. Store.Sources().Get():\n-got/+want\ndiff %s", tt.name, diff) + } + }) + } +}