From e8f433643fa023d6d9375a600db436eb4d3ffc4e Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 4 Nov 2020 10:35:26 -0500 Subject: [PATCH] auth_handler.go: only inject oauth store into handler Previously we were injecting the whole oauth handler chain into this function, which meant we were essentially writing unit tests to test our tests. Let's push some of this logic into the source code. Signed-off-by: Andrew Keesler --- internal/oidc/auth/auth_handler.go | 16 ++++++++++++++-- internal/oidc/auth/auth_handler_test.go | 17 ++--------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index d5016c45..144a8fc1 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -8,7 +8,7 @@ import ( "fmt" "net/http" - "github.com/ory/fosite" + "github.com/ory/fosite/compose" "golang.org/x/oauth2" "k8s.io/klog/v2" @@ -26,11 +26,23 @@ type IDPListGetter interface { func NewHandler( issuer string, idpListGetter IDPListGetter, - oauthHelper fosite.OAuth2Provider, + oauthStore interface{}, generateState func() (state.State, error), generatePKCE func() (pkce.Code, error), generateNonce func() (nonce.Nonce, error), ) http.Handler { + oauthHelper := compose.Compose( + &compose.Config{}, + oauthStore, + &compose.CommonStrategy{ + // Shouldn't need any of this - we aren't doing auth code stuff, issuing ID tokens, or signing + // anything yet. + }, + nil, // hasher, shouldn't need this - we aren't doing any client auth...yet? + compose.OAuth2AuthorizeExplicitFactory, + compose.OpenIDConnectExplicitFactory, + compose.OAuth2PKCEFactory, + ) return httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { if r.Method != http.MethodPost && r.Method != http.MethodGet { // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 09bb192b..1b6282da 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -13,7 +13,6 @@ import ( "testing" "github.com/ory/fosite" - "github.com/ory/fosite/compose" "github.com/ory/fosite/storage" "github.com/stretchr/testify/require" @@ -77,18 +76,6 @@ func TestAuthorizationEndpoint(t *testing.T) { }, }, } - oauthHelper := compose.Compose( - &compose.Config{}, - oauthStore, - &compose.CommonStrategy{ - // Shouldn't need any of this - we aren't doing auth code stuff, issuing ID tokens, or signing - // anything yet. - }, - nil, // hasher, shouldn't need this - we aren't doing any client auth - compose.OAuth2AuthorizeExplicitFactory, // need this to validate generic OAuth2 stuff - compose.OpenIDConnectExplicitFactory, // need this to validate OIDC stuff - compose.OAuth2PKCEFactory, // need this to validate PKCE stuff - ) happyStateGenerator := func() (state.State, error) { return "test-state", nil } happyPKCEGenerator := func() (pkce.Code, error) { return "test-pkce", nil } @@ -340,7 +327,7 @@ func TestAuthorizationEndpoint(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - subject := NewHandler(test.issuer, test.idpListGetter, oauthHelper, test.generateState, test.generatePKCE, test.generateNonce) + subject := NewHandler(test.issuer, test.idpListGetter, oauthStore, test.generateState, test.generatePKCE, test.generateNonce) runOneTestCase(t, test, subject) }) } @@ -349,7 +336,7 @@ func TestAuthorizationEndpoint(t *testing.T) { test := tests[0] require.Equal(t, "happy path using GET", test.name) // re-use the happy path test case - subject := NewHandler(test.issuer, test.idpListGetter, oauthHelper, test.generateState, test.generatePKCE, test.generateNonce) + subject := NewHandler(test.issuer, test.idpListGetter, oauthStore, test.generateState, test.generatePKCE, test.generateNonce) runOneTestCase(t, test, subject)