From 845ec22d5a40a116881d28d52ad8a43755cd2a67 Mon Sep 17 00:00:00 2001 From: "Tomi P. Hakala" Date: Thu, 25 Dec 2025 19:21:35 +0200 Subject: [PATCH 1/2] Fix unchecked errors in gothic package - Check req.ParseForm() error in CompleteUserAuth and return wrapped error - Explicitly ignore fmt.Fprintln error in BeginAuthHandler (terminal error) - Check session.Save errors in tests using a.NoError() Fixes #4 --- gothic/gothic.go | 6 ++++-- gothic/gothic_test.go | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/gothic/gothic.go b/gothic/gothic.go index e97a265a..134ebbdc 100644 --- a/gothic/gothic.go +++ b/gothic/gothic.go @@ -63,7 +63,7 @@ func BeginAuthHandler(res http.ResponseWriter, req *http.Request) { url, err := GetAuthURL(res, req) if err != nil { res.WriteHeader(http.StatusBadRequest) - fmt.Fprintln(res, err) + _, _ = fmt.Fprintln(res, err) return } @@ -194,7 +194,9 @@ var CompleteUserAuth = func(res http.ResponseWriter, req *http.Request) (goth.Us params := req.URL.Query() if params.Encode() == "" && req.Method == "POST" { - req.ParseForm() + if err := req.ParseForm(); err != nil { + return goth.User{}, fmt.Errorf("failed to parse form: %w", err) + } params = req.Form } diff --git a/gothic/gothic_test.go b/gothic/gothic_test.go index f9f17118..4c387b66 100644 --- a/gothic/gothic_test.go +++ b/gothic/gothic_test.go @@ -240,13 +240,13 @@ func Test_StateValidation(t *testing.T) { // Assert that matching states will return a nil error req, _ = http.NewRequest("GET", "/auth/callback?provider=faux&state=state_REAL", nil) - session.Save(req, res) + a.NoError(session.Save(req, res)) _, err = CompleteUserAuth(res, req) a.NoError(err) // Assert that mismatched states will return an error req, _ = http.NewRequest("GET", "/auth/callback?provider=faux&state=state_FAKE", nil) - session.Save(req, res) + a.NoError(session.Save(req, res)) _, err = CompleteUserAuth(res, req) a.Error(err) } From 996afa8e8bd4d1be0ed4c3f2f33acd1a3183b991 Mon Sep 17 00:00:00 2001 From: "Tomi P. Hakala" Date: Thu, 25 Dec 2025 19:40:09 +0200 Subject: [PATCH 2/2] Add test for ParseForm error handling Tests that CompleteUserAuth properly returns an error when req.ParseForm() fails during OAuth callback processing. --- gothic/gothic_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/gothic/gothic_test.go b/gothic/gothic_test.go index 4c387b66..d758fa7e 100644 --- a/gothic/gothic_test.go +++ b/gothic/gothic_test.go @@ -409,3 +409,52 @@ func Test_CompleteUserAuth_SingleSetCookie(t *testing.T) { a.Contains(cookie, "Max-Age=0", "Expected logout cookie with Max-Age=0") } } + +// Test_CompleteUserAuth_ParseFormError verifies that CompleteUserAuth returns +// a proper error when ParseForm fails on a POST request. +func Test_CompleteUserAuth_ParseFormError(t *testing.T) { + a := assert.New(t) + + Store = NewProviderStore() + + // Override GetState to return empty without calling FormValue + // (FormValue internally calls ParseForm, consuming the body before our test can) + originalGetState := GetState + GetState = func(req *http.Request) string { return "" } + defer func() { GetState = originalGetState }() + + // Create a POST request with a body that will cause ParseForm to fail. + req, err := http.NewRequest("POST", "/auth/callback", &errorReader{}) + a.NoError(err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + // Set provider via context so URL query params are empty + // (this triggers the ParseForm code path) + req = GetContextWithProvider(req, "faux") + + // Set up session with provider data but NO AccessToken + // This causes FetchUser to fail, triggering the form parsing path + // Note: Must use the request AFTER GetContextWithProvider since + // ProviderStore keys by request pointer + sess := faux.Session{ID: "test-id", Name: "Test User", Email: "test@example.com"} + session, _ := Store.Get(req, SessionName) + session.Values["faux"] = gzipString(sess.Marshal()) + + res := httptest.NewRecorder() + err = session.Save(req, res) + a.NoError(err) + + // CompleteUserAuth should return an error from ParseForm + // The errorReader body will cause ParseForm to fail + _, err = CompleteUserAuth(res, req) + if a.Error(err, "Expected error from ParseForm failure") { + a.Contains(err.Error(), "failed to parse form", "Error should indicate form parsing failure") + } +} + +// errorReader is an io.Reader that always returns an error +type errorReader struct{} + +func (e *errorReader) Read(p []byte) (n int, err error) { + return 0, fmt.Errorf("simulated read error") +}