From 77a51924a7eff18f2e8f0c7a1632a8f42fc037d7 Mon Sep 17 00:00:00 2001 From: Vikram Rangnekar Date: Sat, 2 Nov 2019 17:13:17 -0400 Subject: [PATCH] Block unauthorized requests when 'anon' role is not defined --- config/allow.list | 17 +++++++++++- config/dev.yml | 3 +-- config/prod.yml | 3 +-- psql/mutate_test.go | 3 --- serv/allow.go | 10 +++---- serv/auth_rails.go | 2 +- serv/cmd.go | 19 +++++-------- serv/config.go | 66 ++++++++++++++++++++++++--------------------- serv/core.go | 6 ++--- serv/http.go | 7 +++-- serv/reload.go | 7 +++++ serv/serv.go | 13 --------- tmpl/dev.yml | 3 +-- tmpl/prod.yml | 3 +-- 14 files changed, 80 insertions(+), 82 deletions(-) diff --git a/config/allow.list b/config/allow.list index 196e46b..f246dea 100644 --- a/config/allow.list +++ b/config/allow.list @@ -159,7 +159,6 @@ query { } } - variables { "data": { "email": "gfk@myspace.com", @@ -272,4 +271,20 @@ query { } } +query { + users { + id + email + picture: avatar + password + full_name + products(limit: 2, where: {price: {gt: 10}}) { + id + name + description + price + } + } +} + diff --git a/config/dev.yml b/config/dev.yml index 497fe0a..77486a5 100644 --- a/config/dev.yml +++ b/config/dev.yml @@ -12,8 +12,7 @@ log_level: "debug" use_allow_list: false # Throw a 401 on auth failure for queries that need auth -# valid values: always, per_query, never -auth_fail_block: never +auth_fail_block: false # Latency tracing for database queries and remote joins # the resulting latency information is returned with the diff --git a/config/prod.yml b/config/prod.yml index 9d5df4c..e44d5ed 100644 --- a/config/prod.yml +++ b/config/prod.yml @@ -16,8 +16,7 @@ log_level: "info" use_allow_list: true # Throw a 401 on auth failure for queries that need auth -# valid values: always, per_query, never -auth_fail_block: always +auth_fail_block: true # Latency tracing for database queries and remote joins # the resulting latency information is returned with the diff --git a/psql/mutate_test.go b/psql/mutate_test.go index a6eaf86..404ba4b 100644 --- a/psql/mutate_test.go +++ b/psql/mutate_test.go @@ -2,7 +2,6 @@ package psql import ( "encoding/json" - "fmt" "testing" ) @@ -261,8 +260,6 @@ func simpleUpdateWithPresets(t *testing.T) { t.Fatal(err) } - fmt.Println(string(resSQL)) - if string(resSQL) != sql { t.Fatal(errNotExpected) } diff --git a/serv/allow.go b/serv/allow.go index f8f02ad..b86d51d 100644 --- a/serv/allow.go +++ b/serv/allow.go @@ -211,13 +211,13 @@ func (al *allowList) save(item *allowItem) { key := gqlHash(item.gql, item.vars, "") - if idx, ok := al.index[key]; ok { - al.list[idx] = item - } else { - al.list = append(al.list, item) - al.index[key] = len(al.list) - 1 + if _, ok := al.index[key]; ok { + return } + al.list = append(al.list, item) + al.index[key] = len(al.list) - 1 + f, err := os.Create(al.filepath) if err != nil { logger.Warn().Err(err).Msgf("Failed to write allow list: %s", al.filepath) diff --git a/serv/auth_rails.go b/serv/auth_rails.go index 7f78da0..4b71d75 100644 --- a/serv/auth_rails.go +++ b/serv/auth_rails.go @@ -121,7 +121,7 @@ func railsCookieHandler(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ck, err := r.Cookie(cookie) - if err != nil { + if err != nil || len(ck.Value) == 0 { logger.Warn().Err(err).Msg("rails cookie missing") next.ServeHTTP(w, r) return diff --git a/serv/cmd.go b/serv/cmd.go index 77730b1..daca612 100644 --- a/serv/cmd.go +++ b/serv/cmd.go @@ -19,20 +19,15 @@ import ( const ( serverName = "Super Graph" - - authFailBlockAlways = iota + 1 - authFailBlockPerQuery - authFailBlockNever ) var ( - logger *zerolog.Logger - conf *config - confPath string - db *pgxpool.Pool - qcompile *qcode.Compiler - pcompile *psql.Compiler - authFailBlock int + logger *zerolog.Logger + conf *config + confPath string + db *pgxpool.Pool + qcompile *qcode.Compiler + pcompile *psql.Compiler ) func Init() { @@ -179,8 +174,6 @@ func initConf() (*config, error) { return nil, fmt.Errorf("unable to decode config, %v", err) } - authFailBlock = getAuthFailBlock(c) - logLevel, err := zerolog.ParseLevel(c.LogLevel) if err != nil { logger.Error().Err(err).Msg("error setting log_level") diff --git a/serv/config.go b/serv/config.go index ba64b8d..254ee5a 100644 --- a/serv/config.go +++ b/serv/config.go @@ -24,7 +24,7 @@ type config struct { EnableTracing bool `mapstructure:"enable_tracing"` UseAllowList bool `mapstructure:"use_allow_list"` WatchAndReload bool `mapstructure:"reload_on_config_change"` - AuthFailBlock string `mapstructure:"auth_fail_block"` + AuthFailBlock bool `mapstructure:"auth_fail_block"` SeedFile string `mapstructure:"seed_file"` MigrationsPath string `mapstructure:"migrations_path"` @@ -103,36 +103,41 @@ type configRemote struct { } `mapstructure:"set_headers"` } +type configQuery struct { + Limit int + Filters []string + Columns []string + DisableFunctions bool `mapstructure:"disable_functions"` + Block bool +} + +type configInsert struct { + Filters []string + Columns []string + Presets map[string]string + Block bool +} + +type configUpdate struct { + Filters []string + Columns []string + Presets map[string]string + Block bool +} + +type configDelete struct { + Filters []string + Columns []string + Block bool +} + type configRoleTable struct { Name string - Query struct { - Limit int - Filters []string - Columns []string - DisableFunctions bool `mapstructure:"disable_functions"` - Block bool - } - - Insert struct { - Filters []string - Columns []string - Presets map[string]string - Block bool - } - - Update struct { - Filters []string - Columns []string - Presets map[string]string - Block bool - } - - Delete struct { - Filters []string - Columns []string - Block bool - } + Query configQuery + Insert configInsert + Update configUpdate + Delete configDelete } type configRole struct { @@ -213,7 +218,7 @@ func (c *config) Init(vi *viper.Viper) error { rolesMap := make(map[string]struct{}) for i := range c.Roles { - role := &c.Roles[i] + role := c.Roles[i] if _, ok := rolesMap[role.Name]; ok { logger.Fatal().Msgf("duplicate role '%s' found", role.Name) @@ -228,7 +233,8 @@ func (c *config) Init(vi *viper.Viper) error { } if _, ok := rolesMap["anon"]; !ok { - c.Roles = append(c.Roles, configRole{Name: "anon"}) + logger.Warn().Msg("unauthenticated requests will be blocked. no role 'anon' defined") + c.AuthFailBlock = true } c.validate() diff --git a/serv/core.go b/serv/core.go index dc5d42b..2fcd35a 100644 --- a/serv/core.go +++ b/serv/core.go @@ -208,10 +208,8 @@ func (c *coreContext) resolveSQL() ([]byte, uint32, error) { buf := &bytes.Buffer{} _, err = t.ExecuteFunc(buf, varMap(c)) - if err == errNoUserID && - authFailBlock == authFailBlockPerQuery && - authCheck(c) == false { - return nil, 0, errUnauthorized + if err == errNoUserID { + logger.Warn().Msg("no user id found. query requires an authenicated request") } if err != nil { diff --git a/serv/http.go b/serv/http.go index 737006d..5cf4e8a 100644 --- a/serv/http.go +++ b/serv/http.go @@ -70,10 +70,9 @@ type resolver struct { func apiv1Http(w http.ResponseWriter, r *http.Request) { ctx := &coreContext{Context: r.Context()} - if authFailBlock == authFailBlockAlways && authCheck(ctx) == false { - err := "Not authorized" - logger.Debug().Msg(err) - http.Error(w, err, 401) + if conf.AuthFailBlock && authCheck(ctx) == false { + w.WriteHeader(http.StatusUnauthorized) + json.NewEncoder(w).Encode(gqlResp{Error: errUnauthorized.Error()}) return } diff --git a/serv/reload.go b/serv/reload.go index b2cc920..4364c79 100644 --- a/serv/reload.go +++ b/serv/reload.go @@ -107,6 +107,13 @@ func Do(log func(string, ...interface{}), additional ...dir) error { case event := <-watcher.Events: // Ensure that we use the correct events, as they are not uniform across // platforms. See https://github.com/fsnotify/fsnotify/issues/74 + + if conf.UseAllowList == false && strings.HasSuffix(event.Name, "/allow.list") { + continue + } + + logger.Info().Msgf("Reloading, file changed detected '%s'", event) + var trigger bool switch runtime.GOOS { case "darwin", "freebsd", "openbsd", "netbsd", "dragonfly": diff --git a/serv/serv.go b/serv/serv.go index 3609daf..55b985b 100644 --- a/serv/serv.go +++ b/serv/serv.go @@ -204,16 +204,3 @@ func getConfigName() string { return ge } - -func getAuthFailBlock(c *config) int { - switch c.AuthFailBlock { - case "always": - return authFailBlockAlways - case "per_query", "perquery", "query": - return authFailBlockPerQuery - case "never", "false": - return authFailBlockNever - } - - return authFailBlockAlways -} diff --git a/tmpl/dev.yml b/tmpl/dev.yml index 64170a1..40b27e8 100644 --- a/tmpl/dev.yml +++ b/tmpl/dev.yml @@ -12,8 +12,7 @@ log_level: "debug" use_allow_list: false # Throw a 401 on auth failure for queries that need auth -# valid values: always, per_query, never -auth_fail_block: never +auth_fail_block: false # Latency tracing for database queries and remote joins # the resulting latency information is returned with the diff --git a/tmpl/prod.yml b/tmpl/prod.yml index d474995..364f0ed 100644 --- a/tmpl/prod.yml +++ b/tmpl/prod.yml @@ -16,8 +16,7 @@ log_level: "info" use_allow_list: true # Throw a 401 on auth failure for queries that need auth -# valid values: always, per_query, never -auth_fail_block: always +auth_fail_block: true # Latency tracing for database queries and remote joins # the resulting latency information is returned with the