diff --git a/CHANGELOG.md b/CHANGELOG.md index 320ba697..2116f8ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.2 +- [#3431](https://github.com/oauth2-proxy/oauth2-proxy/issues/3431) fix: stop logging provider userinfo response body in token validation (@sschmidt) + # V7.15.2 ## Release Highlights diff --git a/providers/internal_util.go b/providers/internal_util.go index 49e1fd94..c6b08914 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -71,12 +71,12 @@ func validateToken(ctx context.Context, p Provider, accessToken string, header h return false } - logger.Printf("%d GET %s %s", result.StatusCode(), stripToken(endpoint), result.Body()) + logger.Printf("%d GET %s", result.StatusCode(), stripToken(endpoint)) if result.StatusCode() == 200 { return true } - logger.Errorf("token validation request failed: status %d - %s", result.StatusCode(), result.Body()) + logger.Errorf("token validation request failed: status %d", result.StatusCode()) return false } diff --git a/providers/internal_util_test.go b/providers/internal_util_test.go index 31952622..4197bbc2 100644 --- a/providers/internal_util_test.go +++ b/providers/internal_util_test.go @@ -1,14 +1,17 @@ package providers import ( + "bytes" "context" "errors" + "io" "net/http" "net/http/httptest" "net/url" "testing" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/stretchr/testify/assert" ) @@ -139,6 +142,29 @@ func TestValidateSessionValidateURLWithQueryParams(t *testing.T) { assert.Equal(t, true, validateToken(context.Background(), vtTest.provider, "foobar", nil)) } +func TestValidateSessionDoesNotLogResponseBody(t *testing.T) { + vtTest := NewValidateSessionTest() + defer vtTest.Close() + + var buf bytes.Buffer + logger.SetOutput(&buf) + logger.SetErrOutput(&buf) + t.Cleanup(func() { + logger.SetOutput(io.Discard) + logger.SetErrOutput(io.Discard) + }) + + // Successful validation must not log the response body. + assert.Equal(t, true, validateToken(context.Background(), vtTest.provider, "foobar", nil)) + assert.NotContains(t, buf.String(), "only code matters; contents disregarded") + + // Error path (non-200) must not log the response body either. + buf.Reset() + vtTest.responseCode = 401 + assert.Equal(t, false, validateToken(context.Background(), vtTest.provider, "foobar", nil)) + assert.NotContains(t, buf.String(), "only code matters; contents disregarded") +} + func TestStripTokenNotPresent(t *testing.T) { test := "http://local.test/api/test?a=1&b=2" assert.Equal(t, test, stripToken(test))