Change error type for redirect parsing errors (#1649)

* Change error type for redirect parsing errors

This changes the error type returned when the proxy fails to parse the
redirect target to be a 400 error instead of a 500 error.

As far as I can tell, the only way that this can fail is a failure to
parse the properties of the request to identity the redirect target.
This indicates that the user has sent a malformed request, and so should
result in a 400 rather than a 500.

I've added a test to exercise this, based on a real work example.

* Update changelog

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -39,6 +39,7 @@ If you are using an architecture specific tag (ex: v7.2.1-arm64) you should move
 - [#1286](https://github.com/oauth2-proxy/oauth2-proxy/pull/1286) Add the `allowed_email_domains` and the `allowed_groups` on the `auth_request` + support standard wildcard char for validation with sub-domain and email-domain. (@w3st3ry @armandpicard)
 - [#1361](https://github.com/oauth2-proxy/oauth2-proxy/pull/1541) PKCE Code Challenge Support - RFC-7636 (@braunsonm)
 - [#1594](https://github.com/oauth2-proxy/oauth2-proxy/pull/1594) Release ARMv8 docker images (@braunsonm)
+- [#1649](https://github.com/oauth2-proxy/oauth2-proxy/pull/1649) Return a 400 instead of a 500 when a request contains an invalid redirect target (@niksko)
 - [#1638](https://github.com/oauth2-proxy/oauth2-proxy/pull/1638) Implement configurable upstream timeout (@jacksgt)
 
 # V7.2.1

oauthproxy.go

@@ -714,7 +714,7 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove
 	appRedirect, err := p.appDirector.GetRedirect(req)
 	if err != nil {
 		logger.Errorf("Error obtaining application redirect: %v", err)
-		p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
+		p.ErrorPage(rw, req, http.StatusBadRequest, err.Error())
 		return
 	}
 

oauthproxy_test.go

@@ -678,6 +678,17 @@ func TestSignInPageIncludesTargetRedirect(t *testing.T) {
 	}
 }
 
+func TestSignInPageInvalidQueryStringReturnsBadRequest(t *testing.T) {
+	sipTest, err := NewSignInPageTest(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	const endpoint = "/?q=%va"
+
+	code, _ := sipTest.GetEndpoint(endpoint)
+	assert.Equal(t, 400, code)
+}
+
 func TestSignInPageDirectAccessRedirectsToRoot(t *testing.T) {
 	sipTest, err := NewSignInPageTest(false)
 	if err != nil {