Skip to content

Fix unchecked errors in gothic package #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tphakala merged 2 commits into from
Dec 25, 2025
Merged

Fix unchecked errors in gothic package #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
845ec22
Fix unchecked errors in gothic package
tphakala Dec 25, 2025
996afa8
Add test for ParseForm error handling
tphakala Dec 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions gothic/gothic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
53 changes: 51 additions & 2 deletions gothic/gothic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,113 +101,113 @@
req, err := http.NewRequest("GET", "/auth?provider=faux", nil)
a.NoError(err)

u, err := GetAuthURL(res, req)
a.NoError(err)

// Check that we get the correct auth URL with a state parameter
parsed, err := url.Parse(u)
a.NoError(err)
a.Equal("http", parsed.Scheme)
a.Equal("example.com", parsed.Host)
q := parsed.Query()
a.Contains(q, "client_id")
a.Equal("code", q.Get("response_type"))
a.NotZero(q, "state")

// Check that if we run GetAuthURL on another request, that request's
// auth URL has a different state from the previous one.
req2, err := http.NewRequest("GET", "/auth?provider=faux", nil)
a.NoError(err)
url2, err := GetAuthURL(httptest.NewRecorder(), req2)
a.NoError(err)

Check warning on line 122 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 18 lines is too similar to gothic/provider_test.go:21
parsed2, err := url.Parse(url2)
a.NoError(err)
a.NotEqual(parsed.Query().Get("state"), parsed2.Query().Get("state"))
}

Check warning on line 127 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 6 lines is too similar to gothic/provider_test.go:39
func Test_CompleteUserAuth(t *testing.T) {
a := assert.New(t)

res := httptest.NewRecorder()
req, err := http.NewRequest("GET", "/auth/callback?provider=faux", nil)
a.NoError(err)

sess := faux.Session{Name: "Homer Simpson", Email: "homer@example.com"}
session, _ := Store.Get(req, SessionName)
session.Values["faux"] = gzipString(sess.Marshal())
err = session.Save(req, res)
a.NoError(err)

user, err := CompleteUserAuth(res, req)
a.NoError(err)

a.Equal(user.Name, "Homer Simpson")
a.Equal(user.Email, "homer@example.com")
}

Check warning on line 147 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 19 lines is too similar to gothic/gothic_test.go:191
func Test_CompleteUserAuthWithSessionDeducedProvider(t *testing.T) {
a := assert.New(t)

Check warning on line 149 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 17 lines is too similar to gothic/gothic_test.go:153

Check warning on line 149 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 16 lines is too similar to gothic/gothic_test.go:176

res := httptest.NewRecorder()
// Intentionally omit a provider argument, force looking in session.
req, err := http.NewRequest("GET", "/auth/callback", nil)
a.NoError(err)

sess := faux.Session{Name: "Homer Simpson", Email: "homer@example.com"}
session, _ := Store.Get(req, SessionName)
session.Values["faux"] = gzipString(sess.Marshal())
err = session.Save(req, res)
a.NoError(err)

user, err := CompleteUserAuth(res, req)
a.NoError(err)

a.Equal(user.Name, "Homer Simpson")
a.Equal(user.Email, "homer@example.com")
}

func Test_CompleteUserAuthWithContextParamProvider(t *testing.T) {
a := assert.New(t)

Check warning on line 170 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 17 lines is too similar to gothic/gothic_test.go:132

res := httptest.NewRecorder()
req, err := http.NewRequest("GET", "/auth/callback", nil)
a.NoError(err)

req = GetContextWithProvider(req, "faux")

sess := faux.Session{Name: "Homer Simpson", Email: "homer@example.com"}
session, _ := Store.Get(req, SessionName)
session.Values["faux"] = gzipString(sess.Marshal())
err = session.Save(req, res)
a.NoError(err)

user, err := CompleteUserAuth(res, req)
a.NoError(err)

a.Equal(user.Name, "Homer Simpson")
a.Equal(user.Email, "homer@example.com")
}

func Test_Logout(t *testing.T) {
a := assert.New(t)

Check warning on line 192 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 16 lines is too similar to gothic/gothic_test.go:133

res := httptest.NewRecorder()
req, err := http.NewRequest("GET", "/auth/callback?provider=faux", nil)
a.NoError(err)

sess := faux.Session{Name: "Homer Simpson", Email: "homer@example.com"}
session, _ := Store.Get(req, SessionName)
session.Values["faux"] = gzipString(sess.Marshal())
err = session.Save(req, res)
a.NoError(err)

user, err := CompleteUserAuth(res, req)
a.NoError(err)

a.Equal(user.Name, "Homer Simpson")
a.Equal(user.Email, "homer@example.com")
err = Logout(res, req)
a.NoError(err)

Check warning on line 210 in gothic/gothic_test.go

View check run for this annotation

Codeac.io / Codeac Code Quality

CodeDuplication 

This block of 19 lines is too similar to gothic/gothic_test.go:128
session, _ = Store.Get(req, SessionName)
a.Equal(session.Values, make(map[interface{}]interface{}))
a.Equal(session.Options.MaxAge, -1)
Expand Down Expand Up @@ -240,13 +240,13 @@

// 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)
}
Expand Down Expand Up @@ -409,3 +409,52 @@
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")
}
Comment on lines +450 to +452
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The testify/assert package provides a convenient ErrorContains function that can simplify this check. It combines asserting that an error is not nil and that its message contains a specific substring into a single call, making the test slightly more concise and readable.

a.ErrorContains(err, "failed to parse form", "Error should indicate form parsing failure")

Copy link
Copy Markdown
Owner Author

@tphakala tphakala Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good suggestion but this project isn't using testify

}

// 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")
}
Loading