From ffdb7fa79501f249fda26b2e95e2f19d07991795 Mon Sep 17 00:00:00 2001
From: Andrew Keesler <akeesler@vmware.com>
Date: Thu, 19 Nov 2020 08:41:44 -0500
Subject: [PATCH] callback_handler.go: add a test for invalid state auth params

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
---
 internal/oidc/callback/callback_handler.go    |  2 +-
 .../oidc/callback/callback_handler_test.go    | 27 ++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go
index cd66f646..2491f20c 100644
--- a/internal/oidc/callback/callback_handler.go
+++ b/internal/oidc/callback/callback_handler.go
@@ -36,7 +36,7 @@ func NewHandler(idpListGetter oidc.IDPListGetter, oauthHelper fosite.OAuth2Provi
 
 		downstreamAuthParams, err := url.ParseQuery(state.AuthParams)
 		if err != nil {
-			panic(err) // TODO
+			return httperr.New(http.StatusBadRequest, "error reading state's downstream auth params")
 		}
 
 		// Recreate enough of the original authorize request so we can pass it to NewAuthorizeRequest().
diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go
index 632ef32b..652b8785 100644
--- a/internal/oidc/callback/callback_handler_test.go
+++ b/internal/oidc/callback/callback_handler_test.go
@@ -96,6 +96,7 @@ func TestCallbackEndpoint(t *testing.T) {
 	happyCSRF := "test-csrf"
 	happyPKCE := "test-pkce"
 	happyNonce := "test-nonce"
+	happyStateVersion := "1"
 
 	happyState, err := happyStateCodec.Encode("s",
 		testutil.ExpectedUpstreamStateParamFormat{
@@ -103,7 +104,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			N: happyNonce,
 			C: happyCSRF,
 			K: happyPKCE,
-			V: "1",
+			V: happyStateVersion,
 		},
 	)
 	require.NoError(t, err)
@@ -114,7 +115,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			N: happyNonce,
 			C: "wrong-csrf-value",
 			K: happyPKCE,
-			V: "1",
+			V: happyStateVersion,
 		},
 	)
 	require.NoError(t, err)
@@ -125,7 +126,18 @@ func TestCallbackEndpoint(t *testing.T) {
 			N: happyNonce,
 			C: happyCSRF,
 			K: happyPKCE,
-			V: "wrong-version",
+			V: "wrong-state-version",
+		},
+	)
+	require.NoError(t, err)
+
+	wrongDownstreamAuthParamsState, err := happyStateCodec.Encode("s",
+		testutil.ExpectedUpstreamStateParamFormat{
+			P: "these-is-not-a-valid-url-query-%z",
+			N: happyNonce,
+			C: happyCSRF,
+			K: happyPKCE,
+			V: happyStateVersion,
 		},
 	)
 	require.NoError(t, err)
@@ -224,6 +236,15 @@ func TestCallbackEndpoint(t *testing.T) {
 			wantStatus:    http.StatusUnprocessableEntity,
 			wantBody:      "Unprocessable Entity: state format version is invalid\n",
 		},
+		{
+			name:          "state's downstream auth params element is invalid",
+			idpListGetter: testutil.NewIDPListGetter(upstreamOIDCIdentityProvider),
+			method:        http.MethodGet,
+			path:          newRequestPath().WithState(wrongDownstreamAuthParamsState).String(),
+			csrfCookie:    happyCSRFCookie,
+			wantStatus:    http.StatusBadRequest,
+			wantBody:      "Bad Request: error reading state's downstream auth params\n",
+		},
 		{
 			name:          "the UpstreamOIDCProvider CRD has been deleted",
 			idpListGetter: testutil.NewIDPListGetter(otherUpstreamOIDCIdentityProvider),