fix: reject whitespace-only tokens in /verify endpoint

[VaibhavAcharya]

Summary

Whitespace-only tokens (e.g. " ", "\t") sent to the /verify endpoint bypass the empty-token check and hit the database, where collation rules can match them against empty confirmation_token/recovery_token fields of confirmed users, leading to authentication bypass and account takeover.

This adds strings.TrimSpace() validation at both the API handler and model layers to reject whitespace-only tokens before they reach the database.

Test plan

Description for the changelog

Fixed authentication bypass via whitespace token in /verify endpoint (SEC-888)

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adds whitespace normalization to token handling across the authentication flow. Files changed: api/external.go trims the invite_token query parameter before use, api/verify.go trims params.Token before validating presence, models/user.go trims tokens in FindUserByConfirmationToken and FindUserByRecoveryToken and returns UserNotFoundError for empty trimmed tokens, and tests were added/updated in api/verify_test.go and models/user_test.go to cover whitespace-only token cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: rejecting whitespace-only tokens in the /verify endpoint, which directly addresses the security vulnerability fixed in this PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The pull request description clearly explains the security vulnerability being fixed (whitespace-only token bypass) and describes the solution (adding strings.TrimSpace validation at API and model layers).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch vaibhavacharya/sec-888-3601607-hackerone-authentication-bypass-on-netlify-identity

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

api/verify_test.go (1)

149-177: Consider adding recovery type variants for completeness (optional).

The tab and multiple-spaces tests only cover the signup type. While the validation occurs before type-specific branching (so coverage is technically sufficient), adding recovery variants for these whitespace patterns would provide symmetric coverage and guard against future refactoring that might change the validation order.

♻️ Optional: Add recovery variants

func (ts *VerifyTestSuite) TestVerify_TabToken_Recovery() {
	var buffer bytes.Buffer
	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
		"type":  "recovery",
		"token": "\t",
	}))

	req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
	req.Header.Set("Content-Type", "application/json")

	w := httptest.NewRecorder()
	ts.API.handler.ServeHTTP(w, req)
	assert.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code)
}

func (ts *VerifyTestSuite) TestVerify_MultipleSpacesToken_Recovery() {
	var buffer bytes.Buffer
	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
		"type":  "recovery",
		"token": "   ",
	}))

	req := httptest.NewRequest(http.MethodPost, "http://localhost/verify", &buffer)
	req.Header.Set("Content-Type", "application/json")

	w := httptest.NewRecorder()
	ts.API.handler.ServeHTTP(w, req)
	assert.Equal(ts.T(), http.StatusUnprocessableEntity, w.Code)
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/verify_test.go` around lines 149 - 177, Add symmetric "recovery" variants
of the existing whitespace token tests to cover both signup and recovery types:
duplicate TestVerify_TabToken and TestVerify_MultipleSpacesToken as
TestVerify_TabToken_Recovery and TestVerify_MultipleSpacesToken_Recovery, set
"type" to "recovery" in their JSON payloads, and call ts.API.handler.ServeHTTP
the same way, asserting http.StatusUnprocessableEntity; update the
VerifyTestSuite tests so both token whitespace cases are validated for the
"recovery" flow to guard against future validation-order changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/verify_test.go`:
- Around line 149-177: Add symmetric "recovery" variants of the existing
whitespace token tests to cover both signup and recovery types: duplicate
TestVerify_TabToken and TestVerify_MultipleSpacesToken as
TestVerify_TabToken_Recovery and TestVerify_MultipleSpacesToken_Recovery, set
"type" to "recovery" in their JSON payloads, and call ts.API.handler.ServeHTTP
the same way, asserting http.StatusUnprocessableEntity; update the
VerifyTestSuite tests so both token whitespace cases are validated for the
"recovery" flow to guard against future validation-order changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5867cbad-065f-474c-936c-255a0a1998e6

📥 Commits

Reviewing files that changed from the base of the PR and between 5431f17 and 8035b95.

📒 Files selected for processing (4)

• api/external.go
• api/verify.go
• api/verify_test.go
• models/user.go

[coderabbitai]

🧹 Nitpick comments (1)

models/user_test.go (1)

89-90: Consider table-driven coverage for more whitespace variants.

This validates " " correctly; adding "\t", "\n", and multi-space inputs here would make model-layer trimming regressions easier to catch.

Suggested test extension

-	_, err = FindUserByConfirmationToken(ts.db, " ")
-	require.EqualError(ts.T(), err, UserNotFoundError{}.Error())
+	for _, token := range []string{" ", "\t", "\n", "   "} {
+		_, err = FindUserByConfirmationToken(ts.db, token)
+		require.EqualError(ts.T(), err, UserNotFoundError{}.Error())
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@models/user_test.go` around lines 89 - 90, The test currently asserts
FindUserByConfirmationToken(ts.db, " ") returns UserNotFoundError only for a
single space; replace that single-case assertion with a small table-driven loop
over whitespace variants (e.g., " ", "\t", "\n", "  ", "\r\n") and call
FindUserByConfirmationToken for each input, asserting require.EqualError(ts.T(),
err, UserNotFoundError{}.Error()) (use ts.T().Run(subcase) if you want subtests)
so the model-layer trimming is exercised across variants; keep the same fixtures
and error expectation but iterate inputs instead of a lone literal.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@models/user_test.go`:
- Around line 89-90: The test currently asserts
FindUserByConfirmationToken(ts.db, " ") returns UserNotFoundError only for a
single space; replace that single-case assertion with a small table-driven loop
over whitespace variants (e.g., " ", "\t", "\n", "  ", "\r\n") and call
FindUserByConfirmationToken for each input, asserting require.EqualError(ts.T(),
err, UserNotFoundError{}.Error()) (use ts.T().Run(subcase) if you want subtests)
so the model-layer trimming is exercised across variants; keep the same fixtures
and error expectation but iterate inputs instead of a lone literal.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 479b0ed8-6e3b-48cc-870f-ed708826b854

📥 Commits

Reviewing files that changed from the base of the PR and between 8035b95 and 393b474.

📒 Files selected for processing (1)

• models/user_test.go