github/wonderwall
1
0
Fork 0
mirror of https://github.com/nais/wonderwall.git synced 2026-05-20 15:22:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(autologin): use glob-style matching instead of regex

Browse Source 
Regexes are powerful, but completely overkill and error-prone for this
use-case. So instead, we'll use path.Match with its simpler glob-style
patterns.
This commit is contained in:
Trong Huu Nguyen
2022-07-21 11:38:37 +02:00
parent 124aff9f08
commit d79f31c18d
6 changed files with 39 additions and 144 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
README.md
View File

@@ -77,7 +77,7 @@ The following flags are available:
```shell
--auto-login Automatically redirect user to login if the user does not have a valid session for all proxied downstream requests.
--auto-login-skip-paths strings Comma separated list of paths to ignore when 'auto-login' is enabled. Paths are evaluated as regular expressions.
--auto-login-skip-paths strings Comma separated list of absolute paths to ignore when 'auto-login' is enabled. Supports basic wildcard matching with glob-style single asterisks using the stdlib path.Match. Invalid patterns are ignored.
--bind-address string Listen address for public connections. (default "127.0.0.1:3000")
--encryption-key string Base64 encoded 256-bit cookie encryption key; must be identical in instances that share session store.
--error-redirect-uri string URI to redirect user to on errors for custom error handling.
@@ -148,7 +148,7 @@ described previously:
### Requirements
- Go 1.17
- Go 1.18
### Binary

48
pkg/autologin/autologin.go
View File

@@ -2,14 +2,15 @@ package autologin
import (
"net/http"
"regexp"
pathlib "path"
"strings"
"github.com/nais/wonderwall/pkg/config"
)
type Options struct {
Enabled bool
SkipRoutes []Route
Enabled bool
SkipPatterns []string
}
func (o *Options) NeedsLogin(r *http.Request, isAuthenticated bool) bool {
@@ -17,8 +18,14 @@ func (o *Options) NeedsLogin(r *http.Request, isAuthenticated bool) bool {
return false
}
for _, route := range o.SkipRoutes {
if route.Regexp.MatchString(r.URL.Path) {
for _, pattern := range o.SkipPatterns {
path := r.URL.Path
if !strings.HasPrefix(path, "/") {
path = "/" + path
}
match, _ := pathlib.Match(pattern, r.URL.Path)
if match {
return false
}
}
@@ -26,36 +33,9 @@ func (o *Options) NeedsLogin(r *http.Request, isAuthenticated bool) bool {
return true
}
type Route struct {
Path string
Regexp *regexp.Regexp
}
func NewOptions(cfg *config.Config) (*Options, error) {
routes, err := skippedRoutes(cfg)
if err != nil {
return nil, err
}
return &Options{
Enabled: cfg.AutoLogin,
SkipRoutes: routes,
Enabled: cfg.AutoLogin,
SkipPatterns: cfg.AutoLoginSkipPaths,
}, nil
}
func skippedRoutes(cfg *config.Config) ([]Route, error) {
routes := make([]Route, 0)
for _, path := range cfg.AutoLoginSkipPaths {
re, err := regexp.Compile(path)
if err != nil {
return nil, err
}
routes = append(routes, Route{
Path: path,
Regexp: re,
})
}
return routes, nil
}

30
pkg/config/config.go
View File

@@ -1,8 +1,6 @@
package config
import (
"fmt"
"regexp"
"time"
"github.com/nais/liberator/pkg/conftools"
@@ -33,29 +31,6 @@ type Config struct {
Loginstatus Loginstatus `json:"loginstatus"`
}
func (in *Config) Validate() error {
if err := in.validateAutoLoginSkipPaths(); err != nil {
return fmt.Errorf("validating '%s': %w", AutoLoginSkipPaths, err)
}
return nil
}
func (in *Config) validateAutoLoginSkipPaths() error {
for _, path := range in.AutoLoginSkipPaths {
if len(path) <= 0 {
return fmt.Errorf("path cannot be empty")
}
_, err := regexp.Compile(path)
if err != nil {
return fmt.Errorf("could not compile regex for path '%s': %w", path, err)
}
}
return nil
}
type Loginstatus struct {
Enabled bool `json:"enabled"`
CookieDomain string `json:"cookie-domain"`
@@ -94,7 +69,7 @@ func Initialize() (*Config, error) {
flag.String(MetricsBindAddress, "127.0.0.1:3001", "Listen address for metrics only.")
flag.Bool(AutoLogin, false, "Automatically redirect user to login if the user does not have a valid session for all proxied downstream requests.")
flag.StringSlice(AutoLoginSkipPaths, []string{}, "Comma separated list of paths to ignore when 'auto-login' is enabled. Paths are evaluated as regular expressions.")
flag.StringSlice(AutoLoginSkipPaths, []string{}, "Comma separated list of absolute paths to ignore when 'auto-login' is enabled. Supports basic wildcard matching with glob-style single asterisks using the stdlib path.Match. Invalid patterns are ignored.")
flag.String(EncryptionKey, "", "Base64 encoded 256-bit cookie encryption key; must be identical in instances that share session store.")
flag.String(ErrorRedirectURI, "", "URI to redirect user to on errors for custom error handling.")
flag.String(Ingress, "", "Ingress used to access the main application.")
@@ -136,9 +111,6 @@ func Initialize() (*Config, error) {
if err := conftools.Load(cfg); err != nil {
return nil, err
}
if err := cfg.Validate(); err != nil {
return nil, err
}
if err := logging.Setup(cfg.LogLevel, cfg.LogFormat); err != nil {
return nil, err

55
pkg/config/config_test.go
View File

@@ -1,55 +0,0 @@
package config_test
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/nais/wonderwall/pkg/config"
)
func TestConfig_Validate(t *testing.T) {
t.Run("auto-login-skip-paths", func(t *testing.T) {
t.Run("valid", func(t *testing.T) {
paths := []string{
"/some/path",
"^/some/path$",
"/some/.+/static/.+$",
}
for _, path := range paths {
t.Run(path, func(t *testing.T) {
cfg := config.Config{
AutoLoginSkipPaths: []string{path},
}
err := cfg.Validate()
assert.NoError(t, err)
})
}
})
t.Run("invalid", func(t *testing.T) {
paths := []string{
"[/some/path",
"^)/some/path$",
"[/some/.*$",
"",
"\\",
"/some/path\\",
"*",
}
for _, path := range paths {
t.Run(path, func(t *testing.T) {
cfg := config.Config{
AutoLoginSkipPaths: []string{path},
}
err := cfg.Validate()
assert.Error(t, err)
})
}
})
})
}

37
pkg/handler/handler_test.go
View File

@@ -231,12 +231,12 @@ func TestHandler_Default(t *testing.T) {
cfg.UpstreamHost = up.URL.Host
cfg.AutoLogin = true
cfg.AutoLoginSkipPaths = []string{
"^/exact/match$",
"^/allowed(/?|/.*)$",
"/partial/(yup|yes)",
"/exact/match",
"/allowed",
"/wildcard/*",
"/deeper/*/*",
"/any*",
}
err := cfg.Validate()
assert.NoError(t, err)
idp := mock.NewIdentityProvider(cfg)
defer idp.Close()
@@ -247,14 +247,12 @@ func TestHandler_Default(t *testing.T) {
matched := []string{
"/exact/match",
"/allowed",
"/allowed/",
"/allowed/very",
"/allowed/very/cool",
"/partial/yes",
"/partial/yup",
"/partial/yes/no",
"/partial/yup/no",
"/parent/partial/yup/no",
"/wildcard/",
"/wildcard/very",
"/deeper/1/",
"/deeper/1/2",
"/anything",
"/anywho",
}
for _, path := range matched {
t.Run(path, func(t *testing.T) {
@@ -273,11 +271,18 @@ func TestHandler_Default(t *testing.T) {
"/",
"/exact/match/",
"/exact/match/huh",
"/allowed/",
"/not-allowed",
"/not-allowed/allowed",
"/alloweded",
"/nope/partial/",
"/nope/partial/child",
"/wildcard",
"/wildcard/yup/nope",
"/deeper",
"/deeper/",
"/deeper/1",
"/deeper/1/2/",
"/deeper/1/2/3",
"/any/",
"/anywho/stvent",
}
for _, path := range nonMatched {
t.Run(path, func(t *testing.T) {

9
pkg/mock/config.go
View File

@@ -8,7 +8,7 @@ import (
)
func Config() *config.Config {
cfg := &config.Config{
return &config.Config{
EncryptionKey: `G8Roe6AcoBpdr5GhO3cs9iORl4XIC8eq`, // 256 bits AES
Ingress: "/",
OpenID: config.OpenID{
@@ -21,13 +21,6 @@ func Config() *config.Config {
},
SessionMaxLifetime: time.Hour,
}
err := cfg.Validate()
if err != nil {
panic(err)
}
return cfg
}
type TestConfiguration struct {