Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: intro preauth client once #723

Merged
merged 8 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions driver/configuration/provider_viper_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func TestViperProvider(t *testing.T) {
assert.True(t, p.AuthenticatorIsEnabled(a.GetID()))
require.NoError(t, a.Validate(nil))

config, err := a.Config(nil)
config, _, err := a.Config(nil)
require.NoError(t, err)
assert.Equal(t, "https://my-website.com/oauth2/introspection", config.IntrospectionURL)
assert.Equal(t, "exact", config.ScopeStrategy)
Expand Down Expand Up @@ -433,7 +433,7 @@ func TestAuthenticatorOAuth2TokenIntrospectionPreAuthorization(t *testing.T) {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
a := authn.NewAuthenticatorOAuth2Introspection(v, logrusx.New("", ""))

config, err := a.Config(json.RawMessage(fmt.Sprintf(`{
config, _, err := a.Config(json.RawMessage(fmt.Sprintf(`{
"pre_authorization": {
"enabled": %v,
"client_id": "%v",
Expand Down
96 changes: 54 additions & 42 deletions pipeline/authn/authenticator_oauth2_introspection.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package authn

import (
"context"
"crypto/md5"
"encoding/json"
"fmt"
"net/http"
"net/url"
"strings"
"sync"
"time"

"github.com/dgraph-io/ristretto"
Expand Down Expand Up @@ -62,16 +64,16 @@ type cacheConfig struct {
type AuthenticatorOAuth2Introspection struct {
c configuration.Provider

client *http.Client
clientMap map[string]*http.Client
mu sync.RWMutex

tokenCache *ristretto.Cache
cacheTTL *time.Duration
logger *logrusx.Logger
}

func NewAuthenticatorOAuth2Introspection(c configuration.Provider, logger *logrusx.Logger) *AuthenticatorOAuth2Introspection {
var rt http.RoundTripper
return &AuthenticatorOAuth2Introspection{c: c, client: httpx.NewResilientClientLatencyToleranceSmall(rt), logger: logger}
return &AuthenticatorOAuth2Introspection{c: c, logger: logger, clientMap: make(map[string]*http.Client)}
}

func (a *AuthenticatorOAuth2Introspection) GetID() string {
Expand Down Expand Up @@ -148,7 +150,7 @@ func (a *AuthenticatorOAuth2Introspection) traceRequest(ctx context.Context, req
}

func (a *AuthenticatorOAuth2Introspection) Authenticate(r *http.Request, session *AuthenticationSession, config json.RawMessage, _ pipeline.Rule) error {
cf, err := a.Config(config)
cf, client, err := a.Config(config)
if err != nil {
return err
}
Expand Down Expand Up @@ -181,7 +183,7 @@ func (a *AuthenticatorOAuth2Introspection) Authenticate(r *http.Request, session
// add tracing
closeSpan := a.traceRequest(r.Context(), introspectReq)

resp, err := a.client.Do(introspectReq.WithContext(r.Context()))
resp, err := client.Do(introspectReq.WithContext(r.Context()))

// close the span so it represents just the http request
closeSpan()
Expand Down Expand Up @@ -252,61 +254,71 @@ func (a *AuthenticatorOAuth2Introspection) Validate(config json.RawMessage) erro
return NewErrAuthenticatorNotEnabled(a)
}

_, err := a.Config(config)
_, _, err := a.Config(config)
return err
}

func (a *AuthenticatorOAuth2Introspection) Config(config json.RawMessage) (*AuthenticatorOAuth2IntrospectionConfiguration, error) {
func (a *AuthenticatorOAuth2Introspection) Config(config json.RawMessage) (*AuthenticatorOAuth2IntrospectionConfiguration, *http.Client, error) {
var c AuthenticatorOAuth2IntrospectionConfiguration
if err := a.c.AuthenticatorConfig(a.GetID(), config, &c); err != nil {
return nil, NewErrAuthenticatorMisconfigured(a, err)
return nil, nil, NewErrAuthenticatorMisconfigured(a, err)
}

var rt http.RoundTripper
clientKey := fmt.Sprintf("%x", md5.Sum([]byte(config)))
a.mu.RLock()
client, ok := a.clientMap[clientKey]
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
a.mu.RUnlock()

if c.PreAuth != nil && c.PreAuth.Enabled {
var ep url.Values
if !ok {
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
a.logger.Debug("Initializing http client")
var rt http.RoundTripper
if c.PreAuth != nil && c.PreAuth.Enabled {
var ep url.Values

if c.PreAuth.Audience != "" {
ep = url.Values{"audience": {c.PreAuth.Audience}}
}
if c.PreAuth.Audience != "" {
ep = url.Values{"audience": {c.PreAuth.Audience}}
}

rt = (&clientcredentials.Config{
ClientID: c.PreAuth.ClientID,
ClientSecret: c.PreAuth.ClientSecret,
Scopes: c.PreAuth.Scope,
EndpointParams: ep,
TokenURL: c.PreAuth.TokenURL,
}).Client(context.Background()).Transport
}
rt = (&clientcredentials.Config{
ClientID: c.PreAuth.ClientID,
ClientSecret: c.PreAuth.ClientSecret,
Scopes: c.PreAuth.Scope,
EndpointParams: ep,
TokenURL: c.PreAuth.TokenURL,
}).Client(context.Background()).Transport
}

if c.Retry == nil {
c.Retry = &AuthenticatorOAuth2IntrospectionRetryConfiguration{Timeout: "500ms", MaxWait: "1s"}
} else {
if c.Retry.Timeout == "" {
c.Retry.Timeout = "500ms"
if c.Retry == nil {
c.Retry = &AuthenticatorOAuth2IntrospectionRetryConfiguration{Timeout: "500ms", MaxWait: "1s"}
} else {
if c.Retry.Timeout == "" {
c.Retry.Timeout = "500ms"
}
if c.Retry.MaxWait == "" {
c.Retry.MaxWait = "1s"
}
}
if c.Retry.MaxWait == "" {
c.Retry.MaxWait = "1s"
duration, err := time.ParseDuration(c.Retry.Timeout)
if err != nil {
return nil, nil, err
}
}
duration, err := time.ParseDuration(c.Retry.Timeout)
if err != nil {
return nil, err
}
timeout := time.Millisecond * duration
timeout := time.Millisecond * duration

maxWait, err := time.ParseDuration(c.Retry.MaxWait)
if err != nil {
return nil, err
}
maxWait, err := time.ParseDuration(c.Retry.MaxWait)
if err != nil {
return nil, nil, err
}

a.client = httpx.NewResilientClientLatencyToleranceConfigurable(rt, timeout, maxWait)
client = httpx.NewResilientClientLatencyToleranceConfigurable(rt, timeout, maxWait)
a.mu.Lock()
a.clientMap[clientKey] = client
a.mu.Unlock()
}

if c.Cache.TTL != "" {
cacheTTL, err := time.ParseDuration(c.Cache.TTL)
if err != nil {
return nil, err
return nil, nil, err
}
a.cacheTTL = &cacheTTL
}
Expand All @@ -325,5 +337,5 @@ func (a *AuthenticatorOAuth2Introspection) Config(config json.RawMessage) (*Auth
a.tokenCache = cache
}

return &c, nil
return &c, client, nil
}
60 changes: 59 additions & 1 deletion pipeline/authn/authenticator_oauth2_introspection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/ory/oathkeeper/internal"
. "github.com/ory/oathkeeper/pipeline/authn"
"github.com/ory/viper"
"github.com/ory/x/logrusx"
)

func TestAuthenticatorOAuth2Introspection(t *testing.T) {
Expand Down Expand Up @@ -556,7 +557,7 @@ func TestAuthenticatorOAuth2Introspection(t *testing.T) {
tc.config, _ = sjson.SetBytes(tc.config, "pre_authorization.token_url", ts.URL+"/oauth2/token")

sess := new(AuthenticationSession)
err := a.Authenticate(tc.r, sess, tc.config, nil)
err = a.Authenticate(tc.r, sess, tc.config, nil)
if tc.expectErr {
require.Error(t, err)
if tc.expectExactErr != nil {
Expand Down Expand Up @@ -589,4 +590,61 @@ func TestAuthenticatorOAuth2Introspection(t *testing.T) {
viper.Set(configuration.ViperKeyAuthenticatorOAuth2TokenIntrospectionIsEnabled, true)
require.Error(t, a.Validate(json.RawMessage(`{"introspection_url":"/oauth2/token"}`)))
})

t.Run("method=config", func(t *testing.T) {
logger := logrusx.New("test", "1")
authenticator := NewAuthenticatorOAuth2Introspection(conf, logger)

noPreauthConfig := []byte(`{ "introspection_url":"http://localhost/oauth2/token" }`)
preAuthConfigOne := []byte(`{ "introspection_url":"http://localhost/oauth2/token","pre_authorization":{"token_url":"http://localhost/oauth2/token","client_id":"some_id","client_secret":"some_secret","enabled":true} }`)
preAuthConfigTwo := []byte(`{ "introspection_url":"http://localhost/oauth2/token2","pre_authorization":{"token_url":"http://localhost/oauth2/token2","client_id":"some_id2","client_secret":"some_secret2","enabled":true} }`)

_, noPreauthClient, err := authenticator.Config(noPreauthConfig)
if err != nil {
require.NoError(t, err)
}

_, preauthOneClient, err := authenticator.Config(preAuthConfigOne)
if err != nil {
require.NoError(t, err)
}

_, preauthTwoClient, err := authenticator.Config(preAuthConfigTwo)
if err != nil {
require.NoError(t, err)
}

require.NotEqual(t, noPreauthClient, preauthOneClient)
require.NotEqual(t, noPreauthClient, preauthTwoClient)
require.NotEqual(t, preauthOneClient, preauthTwoClient)

_, preauthOneClient2, err := authenticator.Config(preAuthConfigOne)
if err != nil {
require.NoError(t, err)
}
if preauthOneClient2 != preauthOneClient {
t.FailNow()
}

_, preauthTwoClient2, err := authenticator.Config(preAuthConfigTwo)
if err != nil {
require.NoError(t, err)
}
if preauthTwoClient2 != preauthTwoClient {
t.FailNow()
}

_, noPreauthClient2, err := authenticator.Config(noPreauthConfig)
if err != nil {
require.NoError(t, err)
}
if noPreauthClient2 != noPreauthClient {
t.FailNow()
}

require.NotEqual(t, noPreauthClient, preauthOneClient)
require.NotEqual(t, noPreauthClient, preauthTwoClient)
require.NotEqual(t, preauthOneClient, preauthTwoClient)

})
}