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 <akeesler@vmware.com>
This commit is contained in:
Andrew Keesler 2020-11-04 10:35:26 -05:00
parent 4f95e6a372
commit e8f433643f
No known key found for this signature in database
GPG Key ID: 27CE0444346F9413
2 changed files with 16 additions and 17 deletions

View File

@ -8,7 +8,7 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"github.com/ory/fosite" "github.com/ory/fosite/compose"
"golang.org/x/oauth2" "golang.org/x/oauth2"
"k8s.io/klog/v2" "k8s.io/klog/v2"
@ -26,11 +26,23 @@ type IDPListGetter interface {
func NewHandler( func NewHandler(
issuer string, issuer string,
idpListGetter IDPListGetter, idpListGetter IDPListGetter,
oauthHelper fosite.OAuth2Provider, oauthStore interface{},
generateState func() (state.State, error), generateState func() (state.State, error),
generatePKCE func() (pkce.Code, error), generatePKCE func() (pkce.Code, error),
generateNonce func() (nonce.Nonce, error), generateNonce func() (nonce.Nonce, error),
) http.Handler { ) 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 { return httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
if r.Method != http.MethodPost && r.Method != http.MethodGet { if r.Method != http.MethodPost && r.Method != http.MethodGet {
// https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

View File

@ -13,7 +13,6 @@ import (
"testing" "testing"
"github.com/ory/fosite" "github.com/ory/fosite"
"github.com/ory/fosite/compose"
"github.com/ory/fosite/storage" "github.com/ory/fosite/storage"
"github.com/stretchr/testify/require" "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 } happyStateGenerator := func() (state.State, error) { return "test-state", nil }
happyPKCEGenerator := func() (pkce.Code, error) { return "test-pkce", nil } happyPKCEGenerator := func() (pkce.Code, error) { return "test-pkce", nil }
@ -340,7 +327,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
for _, test := range tests { for _, test := range tests {
test := test test := test
t.Run(test.name, func(t *testing.T) { 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) runOneTestCase(t, test, subject)
}) })
} }
@ -349,7 +336,7 @@ func TestAuthorizationEndpoint(t *testing.T) {
test := tests[0] test := tests[0]
require.Equal(t, "happy path using GET", test.name) // re-use the happy path test case 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) runOneTestCase(t, test, subject)