Issue 1878: Validate URL call does not correctly honor already set UR… (#1951)

* Issue 1878: Validate URL call does not correctly honor already set URL parameters

* Issue 1878: Validate URL call does not correctly honor already set URL parameters

* Update CHANGELOG.md

---------

Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -14,9 +14,9 @@
 - [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters
 - [#1839](https://github.com/oauth2-proxy/oauth2-proxy/pull/1839) Add readiness checks for deeper health checks (@kobim)
 - [#1927](https://github.com/oauth2-proxy/oauth2-proxy/pull/1927) Fix default scope settings for none oidc providers
+- [#1951](https://github.com/oauth2-proxy/oauth2-proxy/pull/1951) Fix validate URL, check if query string marker (?) or separator (&) needs to be appended (@miguelborges99)
 - [#1920](https://github.com/oauth2-proxy/oauth2-proxy/pull/1920) Make sure emailClaim is not overriden if userIDClaim is not set
 
-
 # V7.4.0
 
 ## Release Highlights

providers/internal_util.go

@@ -53,8 +53,12 @@ func validateToken(ctx context.Context, p Provider, accessToken string, header h
 	endpoint := p.Data().ValidateURL.String()
 	if len(header) == 0 {
 		params := url.Values{"access_token": {accessToken}}
+		if hasQueryParams(endpoint) {
+			endpoint = endpoint + "&" + params.Encode()
+		} else {
 			endpoint = endpoint + "?" + params.Encode()
 		}
+	}
 
 	result := requests.New(endpoint).
 		WithContext(ctx).
@@ -74,3 +78,13 @@ func validateToken(ctx context.Context, p Provider, accessToken string, header h
 	logger.Errorf("token validation request failed: status %d - %s", result.StatusCode(), result.Body())
 	return false
 }
+
+// hasQueryParams check if URL has query parameters
+func hasQueryParams(endpoint string) bool {
+	endpointURL, err := url.Parse(endpoint)
+	if err != nil {
+		return false
+	}
+
+	return len(endpointURL.RawQuery) != 0
+}

providers/internal_util_test.go

@@ -132,6 +132,13 @@ func TestValidateSessionExpiredToken(t *testing.T) {
 	assert.Equal(t, false, validateToken(context.Background(), vtTest.provider, "foobar", nil))
 }
 
+func TestValidateSessionValidateURLWithQueryParams(t *testing.T) {
+	vtTest := NewValidateSessionTest()
+	defer vtTest.Close()
+	vtTest.provider.Data().ValidateURL, _ = url.Parse(vtTest.provider.Data().ValidateURL.String() + "?query_param1=true&query_param2=test")
+	assert.Equal(t, true, validateToken(context.Background(), vtTest.provider, "foobar", nil))
+}
+
 func TestStripTokenNotPresent(t *testing.T) {
 	test := "http://local.test/api/test?a=1&b=2"
 	assert.Equal(t, test, stripToken(test))