From cbc973c8d95658468ff569966aec76dc03ede732 Mon Sep 17 00:00:00 2001 From: Nuno Miguel Micaelo Borges Date: Fri, 10 Feb 2023 18:36:13 +0000 Subject: [PATCH] =?UTF-8?q?Issue=201878:=20Validate=20URL=20call=20does=20?= =?UTF-8?q?not=20correctly=20honor=20already=20set=20UR=E2=80=A6=20(#1951)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Joel Speed --- CHANGELOG.md | 2 +- providers/internal_util.go | 16 +++++++++++++++- providers/internal_util_test.go | 7 +++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88e81b16..6fe5ec52 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/providers/internal_util.go b/providers/internal_util.go index 346ee80d..52cfd0a7 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -53,7 +53,11 @@ 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}} - endpoint = endpoint + "?" + params.Encode() + if hasQueryParams(endpoint) { + endpoint = endpoint + "&" + params.Encode() + } else { + endpoint = endpoint + "?" + params.Encode() + } } result := requests.New(endpoint). @@ -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 +} diff --git a/providers/internal_util_test.go b/providers/internal_util_test.go index 53070017..c8ab3f4f 100644 --- a/providers/internal_util_test.go +++ b/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))