filters/auth: handle trailing question mark in grant flow (#3201)

Do not propagate trailing question mark from the initial request
to redirect URI.

Construct redirect url from scratch instead of resetting URL.ForceQuery.

See related golang/go#13488

Signed-off-by: Alexander Yastrebov <[email protected]>

filters/auth/grant_test.go

@@ -1036,3 +1036,39 @@ func TestGrantLoginRedirectStub(t *testing.T) {
 	assert.Equal(t, int64(len(expectedContent)), rsp.ContentLength)
 	assert.Equal(t, expectedContent, string(body))
 }
+
+func TestGrantLoginRedirectForRequestWithTrailingQuestionMark(t *testing.T) {
+	const (
+		applicationDomain = "foo.skipper.test"
+		callbackPath      = "/a-callback"
+	)
+
+	dnstest.LoopbackNames(t, applicationDomain)
+
+	provider := newGrantTestAuthServer(testToken, testAccessCode)
+	defer provider.Close()
+
+	tokeninfo := newGrantTestTokeninfo(testToken, "")
+	defer tokeninfo.Close()
+
+	config := newGrantTestConfig(tokeninfo.URL, provider.URL)
+	config.CallbackPath = callbackPath
+
+	routes := eskip.MustParse(`* -> oauthGrant() -> status(204) -> <shunt>`)
+
+	proxy, client := newAuthProxy(t, config, routes, applicationDomain)
+	defer proxy.Close()
+
+	// When requested with trailing question mark
+	rsp, err := client.Get(proxy.URL + "?")
+	require.NoError(t, err)
+	defer rsp.Body.Close()
+
+	require.Equal(t, rsp.StatusCode, http.StatusTemporaryRedirect)
+
+	location, err := url.Parse(rsp.Header.Get("Location"))
+	require.NoError(t, err)
+
+	// Then redirect_uri does not contain a trailing question mark
+	assert.Equal(t, proxy.URL+callbackPath, location.Query().Get("redirect_uri"))
+}

filters/auth/grantconfig.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"path/filepath"
 	"strings"
 	"time"
@@ -370,10 +371,11 @@ func (c *OAuthConfig) RedirectURLs(req *http.Request) (redirect, original string
 
 	original = u.String()
 
-	u.Path = c.CallbackPath
-	u.RawQuery = ""
-
-	redirect = u.String()
+	redirect = (&url.URL{
+		Scheme: u.Scheme,
+		Host:   u.Host,
+		Path:   c.CallbackPath,
+	}).String()
 
 	return
 }