From 566f8f231db75d8a558d2526c7fcb20e4cc80107 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 10 Apr 2024 13:58:29 -0500 Subject: [PATCH] chore: add unit test for pass through external auth query params (#12928) * chore: verify pass through external auth query params Unit test added to verify behavior of query params set in the auth url for external apps. This behavior is intended to specifically support Auth0 audience query param. --- coderd/coderdtest/oidctest/idp.go | 4 +- coderd/coderdtest/oidctest/idp_test.go | 6 +- coderd/externalauth/externalauth_test.go | 79 ++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index ff42e31997..c0b95619d4 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -604,7 +604,7 @@ func (f *FakeIDP) CreateAuthCode(t testing.TB, state string) string { // something. // Essentially this is used to fake the Coderd side of the exchange. // The flow starts at the user hitting the OIDC login page. -func (f *FakeIDP) OIDCCallback(t testing.TB, state string, idTokenClaims jwt.MapClaims) (*http.Response, error) { +func (f *FakeIDP) OIDCCallback(t testing.TB, state string, idTokenClaims jwt.MapClaims) *http.Response { t.Helper() if f.serve { panic("cannot use OIDCCallback with WithServing. This is only for the in memory usage") @@ -625,7 +625,7 @@ func (f *FakeIDP) OIDCCallback(t testing.TB, state string, idTokenClaims jwt.Map _ = resp.Body.Close() } }) - return resp, nil + return resp } // ProviderJSON is the .well-known/configuration JSON diff --git a/coderd/coderdtest/oidctest/idp_test.go b/coderd/coderdtest/oidctest/idp_test.go index 519635b067..7706834785 100644 --- a/coderd/coderdtest/oidctest/idp_test.go +++ b/coderd/coderdtest/oidctest/idp_test.go @@ -54,12 +54,12 @@ func TestFakeIDPBasicFlow(t *testing.T) { token = oauthToken }) - resp, err := fake.OIDCCallback(t, expectedState, jwt.MapClaims{}) - require.NoError(t, err) + //nolint:bodyclose + resp := fake.OIDCCallback(t, expectedState, jwt.MapClaims{}) require.Equal(t, http.StatusOK, resp.StatusCode) // Test the user info - _, err = cfg.Provider.UserInfo(ctx, oauth2.StaticTokenSource(token)) + _, err := cfg.Provider.UserInfo(ctx, oauth2.StaticTokenSource(token)) require.NoError(t, err) // Now test it can refresh diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 84fbe4ff5d..88f3b7a3b5 100644 --- a/coderd/externalauth/externalauth_test.go +++ b/coderd/externalauth/externalauth_test.go @@ -3,9 +3,11 @@ package externalauth_test import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" @@ -13,6 +15,7 @@ import ( "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" "golang.org/x/xerrors" @@ -417,6 +420,78 @@ func TestConvertYAML(t *testing.T) { }) } +// TestConstantQueryParams verifies a constant query parameter can be set in the +// "authenticate" url for external auth applications, and it will be carried forward +// to actual auth requests. +// This unit test was specifically created for Auth0 which can set an +// audience query parameter in it's /authorize endpoint. +func TestConstantQueryParams(t *testing.T) { + t.Parallel() + const constantQueryParamKey = "audience" + const constantQueryParamValue = "foobar" + constantQueryParam := fmt.Sprintf("%s=%s", constantQueryParamKey, constantQueryParamValue) + fake, config, _ := setupOauth2Test(t, testConfig{ + FakeIDPOpts: []oidctest.FakeIDPOpt{ + oidctest.WithMiddlewares(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if strings.Contains(request.URL.Path, "authorize") { + // Assert has the audience query param + assert.Equal(t, request.URL.Query().Get(constantQueryParamKey), constantQueryParamValue) + } + next.ServeHTTP(writer, request) + }) + }), + }, + CoderOIDCConfigOpts: []func(cfg *coderd.OIDCConfig){ + func(cfg *coderd.OIDCConfig) { + // Include a constant query parameter. + authURL, err := url.Parse(cfg.OAuth2Config.(*oauth2.Config).Endpoint.AuthURL) + require.NoError(t, err) + + authURL.RawQuery = url.Values{constantQueryParamKey: []string{constantQueryParamValue}}.Encode() + cfg.OAuth2Config.(*oauth2.Config).Endpoint.AuthURL = authURL.String() + require.Contains(t, cfg.OAuth2Config.(*oauth2.Config).Endpoint.AuthURL, constantQueryParam) + }, + }, + }) + + callbackCalled := false + fake.SetCoderdCallbackHandler(func(writer http.ResponseWriter, request *http.Request) { + // Just record the callback was hit, and the auth succeeded. + callbackCalled = true + }) + + // Verify the AuthURL endpoint contains the constant query parameter and is a valid URL. + // It should look something like: + // http://127.0.0.1:>/oauth2/authorize? + // audience=foobar& + // client_id=d& + // redirect_uri=& + // response_type=code& + // scope=openid+email+profile& + // state=state + const state = "state" + rawAuthURL := config.AuthCodeURL(state) + // Parsing the url is not perfect. It allows imperfections like the query + // params having 2 question marks '?a=foo?b=bar'. + // So use it to validate, then verify the raw url is as expected. + authURL, err := url.Parse(rawAuthURL) + require.NoError(t, err) + require.Equal(t, authURL.Query().Get(constantQueryParamKey), constantQueryParamValue) + // We are not using a real server, so it fakes https://coder.com + require.Equal(t, authURL.Scheme, "https") + // Validate the raw URL. + // Double check only 1 '?' exists. Url parsing allows multiple '?' in the query string. + require.Equal(t, strings.Count(rawAuthURL, "?"), 1) + + // Actually run an auth request. Although it says OIDC, the flow is the same + // for oauth2. + //nolint:bodyclose + resp := fake.OIDCCallback(t, state, jwt.MapClaims{}) + require.True(t, callbackCalled) + require.Equal(t, http.StatusOK, resp.StatusCode) +} + type testConfig struct { FakeIDPOpts []oidctest.FakeIDPOpt CoderOIDCConfigOpts []func(cfg *coderd.OIDCConfig) @@ -433,6 +508,10 @@ type testConfig struct { func setupOauth2Test(t *testing.T, settings testConfig) (*oidctest.FakeIDP, *externalauth.Config, database.ExternalAuthLink) { t.Helper() + if settings.ExternalAuthOpt == nil { + settings.ExternalAuthOpt = func(_ *externalauth.Config) {} + } + const providerID = "test-idp" fake := oidctest.NewFakeIDP(t, append([]oidctest.FakeIDPOpt{}, settings.FakeIDPOpts...)...,