Skip to content

Add fail-fast Validate() checks to EmailVerification, PasswordReset, and APIKey handlers#340

Merged
veverkap merged 3 commits into
mainfrom
copilot/fix-missing-validate-methods
May 23, 2026
Merged

Add fail-fast Validate() checks to EmailVerification, PasswordReset, and APIKey handlers#340
veverkap merged 3 commits into
mainfrom
copilot/fix-missing-validate-methods

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 23, 2026

Three handlers in handler/ were missing startup validation while peer handlers already enforce config checks. This left nil dependencies undetected until first live request, including a nil function-pointer panic path in APIKeyHandler.Delete.

  • Validation parity across handlers

    • Added Validate() error to:
      • EmailVerificationHandler (Users, Verifications required)
      • PasswordResetHandler (Users, Resets required)
      • APIKeyHandler (APIKeys, URLParamFunc required)
    • Uses explicit, handler-scoped misconfiguration errors consistent with existing package patterns.
  • Panic-path removal for API key deletion

    • APIKeyHandler now rejects missing URLParamFunc at startup via Validate() instead of failing at runtime on first delete call.
  • Targeted unit coverage

    • Added validation tests for each updated handler:
      • nil required dependency -> error
      • fully configured handler -> no error
func (h *APIKeyHandler) Validate() error {
	if h.APIKeys == nil {
		return errors.New("APIKeyHandler misconfigured: APIKeys is required")
	}
	if h.URLParamFunc == nil {
		return errors.New("APIKeyHandler misconfigured: URLParamFunc is required")
	}
	return nil
}

Greptile Summary

This PR adds Validate() startup checks to three previously unprotected handlers (EmailVerificationHandler, PasswordResetHandler, APIKeyHandler), bringing them in line with peer handlers like MagicLinkHandler. It also adds a belt-and-suspenders nil guard inside APIKeyHandler.Delete() to prevent a runtime panic if the handler is used without first calling Validate().

  • Validate() parity: All three handlers now check every required dependency — including SendEmail and SendResetEmail, which the struct comments already documented as misconfiguration when nil — so a missing field is caught at startup rather than on the first live request.
  • Runtime panic guard: Delete() now checks URLParamFunc != nil before dereferencing it, converting a potential nil-pointer panic into a clean 500 response for any caller that skips Validate().
  • Test coverage: Each Validate() method is covered by individual nil-field tests and a fully-configured happy-path test; the Delete() guard is covered by a dedicated HTTP-recorder test.

Confidence Score: 5/5

Safe to merge — the changes are strictly additive startup checks and a defensive nil guard with no behavioural changes to happy-path request handling.

All three Validate() methods are straightforward nil-checks that mirror the existing MagicLinkHandler pattern. The new runtime guard in Delete() converts a nil-pointer panic into a clean HTTP 500, and every new branch is covered by a targeted unit test. No existing logic is modified.

No files require special attention.

Important Files Changed

Filename Overview
handler/apikey.go Added Validate() checking APIKeys and URLParamFunc, plus a belt-and-suspenders nil guard in Delete() to prevent a runtime panic on unconfigured handlers.
handler/email_verification.go Added Validate() checking Users, Verifications, and SendEmail — now consistent with MagicLinkHandler.Validate() and the struct's own doc comment calling nil SendEmail a misconfiguration.
handler/password_reset.go Added Validate() checking Users, Resets, and SendResetEmail — consistent with peer handlers and the struct doc comment treating nil SendResetEmail as a misconfiguration.
handler/apikey_test.go Adds Validate() tests covering each nil field and a fully-configured happy path, plus a test confirming Delete() returns 500 when URLParamFunc is nil.
handler/email_verification_test.go Adds Validate() tests for nilUsers, nilVerifications, nilSendEmail, and a fully-configured ok path.
handler/password_reset_test.go Adds Validate() tests for nilUsers, nilResets, nilSendResetEmail, and a fully-configured ok path.

Reviews (2): Last reviewed commit: "fix(handlers): add missing Validate chec..." | Re-trigger Greptile

@veverkap veverkap requested review from a team and Copilot May 23, 2026 19:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds fail-fast startup validation to three HTTP handlers in handler/ (EmailVerification, PasswordReset, APIKey) so missing required dependencies are detected at server initialization rather than during the first live request.

Changes:

  • Added Validate() error to EmailVerificationHandler, PasswordResetHandler, and APIKeyHandler to assert required dependencies are non-nil.
  • Added focused unit tests to cover the new validation behavior for each handler.
Show a summary per file
File Description
handler/password_reset.go Adds PasswordResetHandler.Validate() to fail fast when Users/Resets are nil.
handler/password_reset_test.go Adds tests for password-reset handler validation success/failure cases.
handler/email_verification.go Adds EmailVerificationHandler.Validate() to fail fast when Users/Verifications are nil.
handler/email_verification_test.go Adds tests for email-verification handler validation success/failure cases.
handler/apikey.go Adds APIKeyHandler.Validate() to fail fast when APIKeys/URLParamFunc are nil.
handler/apikey_test.go Adds tests for API key handler validation success/failure cases.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 1

Comment thread handler/apikey.go
Comment thread handler/email_verification.go
Comment thread handler/password_reset.go
- EmailVerificationHandler.Validate() now checks SendEmail is set
- PasswordResetHandler.Validate() now checks SendResetEmail is set
- APIKeyHandler.Delete() returns 500 when URLParamFunc is nil instead of panicking
- Add tests for all three new validation/guard paths
@veverkap veverkap merged commit 329c1a5 into main May 23, 2026
53 checks passed
@veverkap veverkap deleted the copilot/fix-missing-validate-methods branch May 23, 2026 20:02
github-actions Bot added a commit that referenced this pull request May 23, 2026
…n, and PasswordReset handlers

Add Validate() call examples and descriptions to the Configuration
sections of api-keys.md, email-verification.md, and password-reset.md,
matching the existing pattern in magic-links.md and totp.md.

Triggered by #340 which added Validate() to these three handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
veverkap added a commit that referenced this pull request May 23, 2026
* docs: document Validate() startup checks for APIKey, EmailVerification, and PasswordReset handlers

Add Validate() call examples and descriptions to the Configuration
sections of api-keys.md, email-verification.md, and password-reset.md,
matching the existing pattern in magic-links.md and totp.md.

Triggered by #340 which added Validate() to these three handlers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* docs(handler): fix contradictory Validate() field lists and remove duplicate code blocks

email-verification.md and password-reset.md each had two descriptions of
Validate() that disagreed on which fields are required. The inline note
omitted SendEmail/SendResetEmail, the secondary paragraph was correct but
included a duplicate code block. Consolidate into a single accurate
description listing all three required fields with the descriptive error
message format.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Patrick Veverka <veverkap@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants