diff --git a/cmd/skipper/main.go b/cmd/skipper/main.go index cb1a5a6354..1b844bbf52 100644 --- a/cmd/skipper/main.go +++ b/cmd/skipper/main.go @@ -57,6 +57,9 @@ const ( defaultTLSHandshakeTimeoutBackend = 60 * time.Second defaultMaxIdleConnsBackend = 0 + // OAuth2: + defaultOAuthTokeninfoTimeout = 2 * time.Second // Not sure if this makes sense + // generic: addressUsage = "network address that skipper should listen on" ignoreTrailingSlashUsage = "flag indicating to ignore trailing slashes in paths when routing" @@ -128,10 +131,11 @@ const ( kubernetesNamespaceUsage = "watch only this namespace for ingresses" // OAuth2: - oauthURLUsage = "OAuth2 URL for Innkeeper authentication" - oauthCredentialsDirUsage = "directory where oauth credentials are stored: client.json and user.json" - oauthScopeUsage = "the whitespace separated list of oauth scopes" - oauth2TokeninfoURLUsage = "sets the default tokeninfo URL to query information about an incoming OAuth2 token in oauth2Tokeninfo filters" + oauthURLUsage = "OAuth2 URL for Innkeeper authentication" + oauthCredentialsDirUsage = "directory where oauth credentials are stored: client.json and user.json" + oauthScopeUsage = "the whitespace separated list of oauth scopes" + oauth2TokeninfoURLUsage = "sets the default tokeninfo URL to query information about an incoming OAuth2 token in oauth2Tokeninfo filters" + oauth2TokeninfoTimeoutUsage = "sets the default tokeninfo request timeout duration to 1500ms" // connections, timeouts: idleConnsPerHostUsage = "maximum idle connections per backend host" @@ -235,10 +239,11 @@ var ( kubernetesNamespace string // OAuth2: - oauthURL string - oauthScope string - oauthCredentialsDir string - oauth2TokeninfoURL string + oauthURL string + oauthScope string + oauthCredentialsDir string + oauth2TokeninfoURL string + oauth2TokeninfoTimeout time.Duration // connections, timeouts: idleConnsPerHost int @@ -344,6 +349,7 @@ func init() { flag.StringVar(&oauthScope, "oauth-scope", "", oauthScopeUsage) flag.StringVar(&oauthCredentialsDir, "oauth-credentials-dir", "", oauthCredentialsDirUsage) flag.StringVar(&oauth2TokeninfoURL, "oauth2-tokeninfo-url", "", oauth2TokeninfoURLUsage) + flag.DurationVar(&oauth2TokeninfoTimeout, "oauth2-tokeninfo-timeout", defaultOAuthTokeninfoTimeout, oauth2TokeninfoTimeoutUsage) // connections, timeouts: flag.IntVar(&idleConnsPerHost, "idle-conns-num", proxy.DefaultIdleConnsPerHost, idleConnsPerHostUsage) @@ -499,10 +505,11 @@ func main() { KubernetesNamespace: kubernetesNamespace, // OAuth2: - OAuthUrl: oauthURL, - OAuthScope: oauthScope, - OAuthCredentialsDir: oauthCredentialsDir, - OAuthTokeninfoURL: oauth2TokeninfoURL, + OAuthUrl: oauthURL, + OAuthScope: oauthScope, + OAuthCredentialsDir: oauthCredentialsDir, + OAuthTokeninfoURL: oauth2TokeninfoURL, + OAuthTokeninfoTimeout: oauth2TokeninfoTimeout, // connections, timeouts: IdleConnectionsPerHost: idleConnsPerHost, diff --git a/filters/auth/doc.go b/filters/auth/doc.go index 787f82a6cc..7af143a69f 100644 --- a/filters/auth/doc.go +++ b/filters/auth/doc.go @@ -39,6 +39,10 @@ TokeninfoURL. The request from skipper to TokeninfoURL will use the query string to do the request: ?access_token=. +Additionally, you can also pass CLI argument +-oauth2-tokeninfo-timeout= to control the default +timeout duration for OAuth validation request + Example json output of the tokeninfo response could be: { @@ -116,7 +120,7 @@ the key value pairs match. Here "uid" has to have the value "jdoe" and "foo" has to have the value "bar". Additionally the second will check if there is a "realm" "/employees": - a: Path("/") -> oauthTokeninfoAllKV("uid", jdoe", "foo", "bar") -> "https://internal.example.org/"; + a: Path("/") -> oauthTokeninfoAllKV("uid", "jdoe", "foo", "bar") -> "https://internal.example.org/"; b: Path("/") -> oauthTokeninfoAllKV("realm", "/employees", "uid", "jdoe", "foo", "bar") -> "https://internal.example.org/"; Example json output of this information response: @@ -140,6 +144,11 @@ Example json output of this information response: } +In case you are using any of the above 4 filters in your custom build, +you can call the `Close()` method to close the `quit` channel and +free up goroutines, to avoid goroutine leak + + OAuth - auditLog() filter The filter auditLog allows you to have an audit log for all diff --git a/filters/auth/oauth.go b/filters/auth/oauth.go index 6bc9a72096..b159df9cfc 100644 --- a/filters/auth/oauth.go +++ b/filters/auth/oauth.go @@ -4,9 +4,11 @@ import ( "encoding/json" "errors" "fmt" + "net" "net/http" "net/url" "strings" + "time" log "github.com/sirupsen/logrus" "github.com/zalando/skipper/filters" @@ -49,13 +51,16 @@ const ( type ( authClient struct { - url *url.URL + url *url.URL + client *http.Client + quit chan struct{} } tokeninfoSpec struct { - typ roleCheckType - tokeninfoURL string - authClient *authClient + typ roleCheckType + tokeninfoURL string + tokenInfoTimeout time.Duration + authClient *authClient } filter struct { @@ -145,9 +150,32 @@ func intersect(left []string, right []string) bool { return false } +func createHTTPClient(timeout time.Duration, quit chan struct{}) (*http.Client, error) { + transport := &http.Transport{ + DialContext: (&net.Dialer{ + Timeout: timeout, + }).DialContext, + } + + go func() { + for { + select { + case <-time.After(10 * time.Second): + transport.CloseIdleConnections() + case <-quit: + return + } + } + }() + + return &http.Client{ + Transport: transport, + }, nil +} + // jsonGet requests url with access token in the URL query specified // by accessTokenQueryKey, if auth was given and writes into doc. -func jsonGet(url *url.URL, auth string, doc interface{}) error { +func jsonGet(url *url.URL, auth string, doc interface{}, client *http.Client) error { if auth != "" { q := url.Query() q.Set(accessTokenQueryKey, auth) @@ -159,7 +187,7 @@ func jsonGet(url *url.URL, auth string, doc interface{}) error { return err } - rsp, err := http.DefaultClient.Do(req) + rsp, err := client.Do(req) if err != nil { return err } @@ -173,17 +201,23 @@ func jsonGet(url *url.URL, auth string, doc interface{}) error { return d.Decode(doc) } -func newAuthClient(baseURL string) (*authClient, error) { +func newAuthClient(baseURL string, timeout time.Duration) (*authClient, error) { u, err := url.Parse(baseURL) if err != nil { return nil, err } - return &authClient{url: u}, nil + + quit := make(chan struct{}) + client, err := createHTTPClient(timeout, quit) + if err != nil { + log.Error("Unable to create http client") + } + return &authClient{url: u, client: client, quit: quit}, nil } func (ac *authClient) getTokeninfo(token string) (map[string]interface{}, error) { var a map[string]interface{} - err := jsonGet(ac.url, token, &a) + err := jsonGet(ac.url, token, &a, ac.client) return a, err } @@ -191,32 +225,32 @@ func (ac *authClient) getTokeninfo(token string) (map[string]interface{}, error) // to validate authorization for requests. Current implementation uses // Bearer tokens to authorize requests and checks that the token // contains all scopes. -func NewOAuthTokeninfoAllScope(OAuthTokeninfoURL string) filters.Spec { - return &tokeninfoSpec{typ: checkOAuthTokeninfoAllScopes, tokeninfoURL: OAuthTokeninfoURL} +func NewOAuthTokeninfoAllScope(OAuthTokeninfoURL string, OAuthTokeninfoTimeout time.Duration) filters.Spec { + return &tokeninfoSpec{typ: checkOAuthTokeninfoAllScopes, tokeninfoURL: OAuthTokeninfoURL, tokenInfoTimeout: OAuthTokeninfoTimeout} } // NewOAuthTokeninfoAnyScope creates a new auth filter specification // to validate authorization for requests. Current implementation uses // Bearer tokens to authorize requests and checks that the token // contains at least one scope. -func NewOAuthTokeninfoAnyScope(OAuthTokeninfoURL string) filters.Spec { - return &tokeninfoSpec{typ: checkOAuthTokeninfoAnyScopes, tokeninfoURL: OAuthTokeninfoURL} +func NewOAuthTokeninfoAnyScope(OAuthTokeninfoURL string, OAuthTokeninfoTimeout time.Duration) filters.Spec { + return &tokeninfoSpec{typ: checkOAuthTokeninfoAnyScopes, tokeninfoURL: OAuthTokeninfoURL, tokenInfoTimeout: OAuthTokeninfoTimeout} } // NewOAuthTokeninfoAllKV creates a new auth filter specification // to validate authorization for requests. Current implementation uses // Bearer tokens to authorize requests and checks that the token // contains all key value pairs provided. -func NewOAuthTokeninfoAllKV(OAuthTokeninfoURL string) filters.Spec { - return &tokeninfoSpec{typ: checkOAuthTokeninfoAllKV, tokeninfoURL: OAuthTokeninfoURL} +func NewOAuthTokeninfoAllKV(OAuthTokeninfoURL string, OAuthTokeninfoTimeout time.Duration) filters.Spec { + return &tokeninfoSpec{typ: checkOAuthTokeninfoAllKV, tokeninfoURL: OAuthTokeninfoURL, tokenInfoTimeout: OAuthTokeninfoTimeout} } // NewOAuthTokeninfoAnyKV creates a new auth filter specification // to validate authorization for requests. Current implementation uses // Bearer tokens to authorize requests and checks that the token // contains at least one key value pair provided. -func NewOAuthTokeninfoAnyKV(OAuthTokeninfoURL string) filters.Spec { - return &tokeninfoSpec{typ: checkOAuthTokeninfoAnyKV, tokeninfoURL: OAuthTokeninfoURL} +func NewOAuthTokeninfoAnyKV(OAuthTokeninfoURL string, OAuthTokeninfoTimeout time.Duration) filters.Spec { + return &tokeninfoSpec{typ: checkOAuthTokeninfoAnyKV, tokeninfoURL: OAuthTokeninfoURL, tokenInfoTimeout: OAuthTokeninfoTimeout} } func (s *tokeninfoSpec) Name() string { @@ -240,7 +274,7 @@ func (s *tokeninfoSpec) Name() string { // type. The shown example for checkOAuthTokeninfoAllScopes will grant // access only to tokens, that have scopes read-x and write-y: // -// s.CreateFilter(read-x", "write-y") +// s.CreateFilter("read-x", "write-y") // func (s *tokeninfoSpec) CreateFilter(args []interface{}) (filters.Filter, error) { sargs, err := getStrings(args) @@ -251,7 +285,7 @@ func (s *tokeninfoSpec) CreateFilter(args []interface{}) (filters.Filter, error) return nil, filters.ErrInvalidFilterParameters } - ac, err := newAuthClient(s.tokeninfoURL) + ac, err := newAuthClient(s.tokeninfoURL, s.tokenInfoTimeout) if err != nil { return nil, filters.ErrInvalidFilterParameters } @@ -433,3 +467,10 @@ func (f *filter) Request(ctx filters.FilterContext) { } func (f *filter) Response(filters.FilterContext) {} + +// Close cleans-up the quit channel used for this spec +func (f *filter) Close() { + if f.authClient.quit != nil { + close(f.authClient.quit) + } +} diff --git a/filters/auth/oauth_test.go b/filters/auth/oauth_test.go index daa49c5972..afa41ddcad 100644 --- a/filters/auth/oauth_test.go +++ b/filters/auth/oauth_test.go @@ -7,6 +7,7 @@ import ( "net/url" "strings" "testing" + "time" "github.com/zalando/skipper/eskip" "github.com/zalando/skipper/filters" @@ -14,16 +15,17 @@ import ( ) const ( - testToken = "test-token" - testUID = "jdoe" - testScope = "test-scope" - testScope2 = "test-scope2" - testScope3 = "test-scope3" - testRealmKey = "/realm" - testRealm = "/immortals" - testKey = "uid" - testValue = "jdoe" - testAuthPath = "/test-auth" + testToken = "test-token" + testUID = "jdoe" + testScope = "test-scope" + testScope2 = "test-scope2" + testScope3 = "test-scope3" + testRealmKey = "/realm" + testRealm = "/immortals" + testKey = "uid" + testValue = "jdoe" + testAuthPath = "/test-auth" + testAuthTimeout = 100 * time.Millisecond ) type testAuthDoc struct { @@ -305,24 +307,32 @@ func TestOAuth2Tokeninfo(t *testing.T) { } })) - var s filters.Spec + var spec filters.Spec args := []interface{}{} u := authServer.URL + ti.authBaseURL switch ti.authType { case OAuthTokeninfoAnyScopeName: - s = NewOAuthTokeninfoAnyScope(u) + spec = NewOAuthTokeninfoAnyScope(u, testAuthTimeout) case OAuthTokeninfoAllScopeName: - s = NewOAuthTokeninfoAllScope(u) + spec = NewOAuthTokeninfoAllScope(u, testAuthTimeout) case OAuthTokeninfoAnyKVName: - s = NewOAuthTokeninfoAnyKV(u) + spec = NewOAuthTokeninfoAnyKV(u, testAuthTimeout) case OAuthTokeninfoAllKVName: - s = NewOAuthTokeninfoAllKV(u) + spec = NewOAuthTokeninfoAllKV(u, testAuthTimeout) } args = append(args, ti.args...) + f, err := spec.CreateFilter(args) + if err != nil { + t.Logf("error in creating filter") + return + } + f2 := f.(*filter) + defer f2.Close() + fr := make(filters.Registry) - fr.Register(s) - r := &eskip.Route{Filters: []*eskip.Filter{{Name: s.Name(), Args: args}}, Backend: backend.URL} + fr.Register(spec) + r := &eskip.Route{Filters: []*eskip.Filter{{Name: spec.Name(), Args: args}}, Backend: backend.URL} proxy := proxytest.New(fr, r) reqURL, err := url.Parse(proxy.URL) @@ -364,3 +374,103 @@ func TestOAuth2Tokeninfo(t *testing.T) { }) } } + +func TestOAuth2TokenTimeout(t *testing.T) { + for _, ti := range []struct { + msg string + timeout time.Duration + auth string + authType string + expected int + }{{ + msg: "get token within specified timeout", + timeout: 2 * testAuthTimeout, + authType: OAuthTokeninfoAnyScopeName, + expected: http.StatusOK, + }, { + msg: "get token request timeout", + timeout: 50 * time.Millisecond, + authType: OAuthTokeninfoAnyScopeName, + expected: http.StatusUnauthorized, + }} { + t.Run(ti.msg, func(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {})) + + handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != testAuthPath { + w.WriteHeader(http.StatusNotFound) + return + } + + token, err := getToken(r) + if err != nil || token != testToken { + w.WriteHeader(http.StatusUnauthorized) + return + } + + d := map[string]interface{}{ + "uid": testUID, + testRealmKey: testRealm, + "scope": []string{testScope}, + } + + time.Sleep(100 * time.Millisecond) + e := json.NewEncoder(w) + err = e.Encode(&d) + if err != nil { + t.Error(err) + } + }) + authServer := httptest.NewServer(http.TimeoutHandler(handlerFunc, ti.timeout, "server unavailable")) + + args := []interface{}{testScope} + u := authServer.URL + testAuthPath + spec := NewOAuthTokeninfoAnyScope(u, ti.timeout) + + scopes := []interface{}{"read-x"} + f, err := spec.CreateFilter(scopes) + if err != nil { + t.Error(err) + return + } + f2 := f.(*filter) + defer f2.Close() + + fr := make(filters.Registry) + fr.Register(spec) + r := &eskip.Route{Filters: []*eskip.Filter{{Name: spec.Name(), Args: args}}, Backend: backend.URL} + + proxy := proxytest.New(fr, r) + reqURL, err := url.Parse(proxy.URL) + if err != nil { + t.Errorf("Failed to parse url %s: %v", proxy.URL, err) + } + + q := reqURL.Query() + q.Add(accessTokenQueryKey, testToken) + reqURL.RawQuery = q.Encode() + + req, err := http.NewRequest("GET", reqURL.String(), nil) + + if err != nil { + t.Error(err) + return + } + + resp, err := http.DefaultClient.Do(req) + + if err != nil { + t.Error(err) + return + } + + defer resp.Body.Close() + + if resp.StatusCode != ti.expected { + t.Errorf("auth filter failed got=%d, expected=%d, route=%s", resp.StatusCode, ti.expected, r) + buf := make([]byte, resp.ContentLength) + resp.Body.Read(buf) + } + }) + } +} diff --git a/readme.md b/readme.md index 50fd79ec92..dcbf0a555c 100644 --- a/readme.md +++ b/readme.md @@ -104,10 +104,10 @@ Getting the code with the test dependencies (`-t` switch): Build and test all packages: - cd src/github.com/zalando/skipper - make deps - make install - make shortcheck + cd src/github.com/zalando/skipper + make deps + make install + make shortcheck #### Kubernetes Ingress diff --git a/skipper.go b/skipper.go index 6ef26b5a65..827742c6b0 100644 --- a/skipper.go +++ b/skipper.go @@ -433,6 +433,9 @@ type Options struct { // OAuthTokeninfoURL sets the OAuthTokeninfoURL similar to https://godoc.org/golang.org/x/oauth2#Endpoint OAuthTokeninfoURL string + // OAuthTokeninfoTimeout sets timeout duration while calling oauth token service + OAuthTokeninfoTimeout time.Duration + // MaxAuditBody sets the maximum read size of the body read by the audit log filter MaxAuditBody int } @@ -655,10 +658,10 @@ func Run(o Options) error { } if o.OAuthTokeninfoURL != "" { - o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAllScope(o.OAuthTokeninfoURL)) - o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAnyScope(o.OAuthTokeninfoURL)) - o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAllKV(o.OAuthTokeninfoURL)) - o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAnyKV(o.OAuthTokeninfoURL)) + o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAllScope(o.OAuthTokeninfoURL, o.OAuthTokeninfoTimeout)) + o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAnyScope(o.OAuthTokeninfoURL, o.OAuthTokeninfoTimeout)) + o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAllKV(o.OAuthTokeninfoURL, o.OAuthTokeninfoTimeout)) + o.CustomFilters = append(o.CustomFilters, auth.NewOAuthTokeninfoAnyKV(o.OAuthTokeninfoURL, o.OAuthTokeninfoTimeout)) } o.CustomFilters = append(o.CustomFilters, logfilter.NewAuditLog(o.MaxAuditBody))