docs: add PasskeyHandler error response reference table#51
Conversation
Document all non-200 HTTP status codes returned by PasskeyHandler endpoints, including the 500 path in FinishRegistration when Users.FindByID fails (fixed in #48 to prevent a nil pointer panic). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
veverkananobot
left a comment
There was a problem hiding this comment.
✅ APPROVED
Excellent documentation follow-up to PR #48: comprehensive error response reference table for all PasskeyHandler endpoints.
Summary
Documentation-only PR adding an Error responses subsection to the PasskeyHandler README section. New table documents all non-200 status codes for each endpoint:
- 7 public endpoints (Enabled, BeginRegistration, FinishRegistration, BeginAuthentication, FinishAuthentication, ListCredentials, DeleteCredential)
- 23 error paths with status codes, conditions, and causes
Verification Against Source (handler/passkey.go)
✅ All endpoints present and correctly named (verified against function signatures):
- Enabled, BeginRegistration, FinishRegistration, BeginAuthentication, FinishAuthentication, ListCredentials, DeleteCredential
✅ Status codes verified:
503 Service Unavailable— All endpoints check WebAuthn nil first ✓400 Bad Request— Input validation errors (name length, session_id missing, session expired/invalid, verification failed) ✓401 Unauthorized— FinishAuthentication only (session/assertion failure) ✓404 Not Found— DeleteCredential only (credential not found) ✓500 Internal Server Error— Store/ceremony errors across all endpoints ✓
✅ Error conditions match source exactly:
- BeginRegistration 400:
name required (max 100 chars)✓ - BeginRegistration 500: user lookup, ceremony, challenge storage ✓
- FinishRegistration 400: session_id missing/invalid/expired/different-user, verification failed ✓
- FinishRegistration 500: user lookup (PR #48 fix), credential storage ✓
- BeginAuthentication 500: ceremony, challenge storage ✓
- FinishAuthentication 400: session_id missing ✓
- FinishAuthentication 401: session/assertion failure ✓
- FinishAuthentication 500: JWT creation ✓
- ListCredentials 500: store error ✓
- DeleteCredential 400: credential ID missing ✓
- DeleteCredential 404: credential not found ✓
- DeleteCredential 500: store error ✓
Quality Assessment
✅ Completeness: All 7 endpoints covered with all error paths documented
✅ Accuracy: All status codes and conditions verified line-by-line against source
✅ Clarity: Condition column clearly explains when each error occurs
✅ Context: Properly motivated by PR #48's nil pointer fix introducing new 500 path
✅ Scope: All passkey endpoints, no other handlers
✅ No breaking changes: Documentation-only, zero code changes
Why This Matters
Before PR #48, FinishRegistration silently discarded errors and panicked. The fix propagates errors and returns 500. Without this table, API consumers have no clear contract for:
- What status codes to expect
- When transient errors (store failures) occur
- How to distinguish validation errors (400) from server errors (500)
This closes that gap for all passkey endpoints.
CI Status
Tests running (Go Lint, Analyze (go), Go Tests in progress; Analyze (actions) SUCCESS, CodeQL NEUTRAL, Greptile in progress).
Recommendation: Merge when CI passes. Solid reference documentation for passkey integration.
There was a problem hiding this comment.
Pull request overview
Updates the README’s PasskeyHandler (WebAuthn) documentation to reflect the full set of error responses callers may see, including the newly documented 500 path introduced after #48 fixed FinishRegistration’s FindByID error handling.
Changes:
- Added an “Error responses” subsection under PasskeyHandler – WebAuthn.
- Introduced a table enumerating non-200 status codes per passkey endpoint with conditions.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a PasskeyHandler error-response reference table for passkey endpoints. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
README.md:616
FinishRegistrationmaps any challenge-load failure (including challenge store errors or unmarshal failures) to400 Bad Request("invalid or expired session"). The 400 condition currently only lists missing/not found/expired/different user; consider mentioning challenge lookup failures (store/unmarshal) here as well, since they also produce 400.
| `FinishRegistration` | `400 Bad Request` | `session_id` query parameter missing, session not found, session expired, or session belongs to a different user |
| `FinishRegistration` | `400 Bad Request` | WebAuthn attestation verification failed |
README.md:621
FinishAuthenticationreturns401 Unauthorizednot only for missing/expired sessions and assertion verification failures, but also when credential/user lookup inside the WebAuthn discoverable user handler fails (those errors bubble up and are surfaced as "authentication failed"). To keep this table accurate, consider broadening the 401 condition to include credential/user lookup failures (store errors or not found).
| `FinishAuthentication` | `401 Unauthorized` | Session not found, session expired, or WebAuthn assertion verification failed |
| `FinishAuthentication` | `500 Internal Server Error` | JWT creation failed |
- Files reviewed: 1/1 changed files
- Comments generated: 2
Triggered by the merge of #48, which fixed a nil pointer panic in
FinishRegistrationwhenUsers.FindByIDreturns an error. That fix introduced a new HTTP 500 path that was not reflected in the documentation.Changes
Adds an "Error responses" subsection to the
PasskeyHandler – WebAuthnsection of the README. The new table lists every non-200 status code each passkey endpoint can produce, including:503 Service Unavailable—WebAuthnfield isnil400 Bad Request— invalid input or failed WebAuthn ceremony401 Unauthorized— authentication failure404 Not Found— credential not found500 Internal Server Error— store errors, including theFinishRegistrationuser-lookup failure fixed in Fix nil pointer panic in FinishRegistration when FindByID fails #48Why this matters
Before #48,
FinishRegistrationsilently discarded theFindByIDerror and panicked. The fix propagates the error and returns500. Without documentation, callers have no clear contract for what to expect when the user store is temporarily unavailable. This table closes that gap for all passkey endpoints.Testing
Documentation-only change. No code changes were made.
Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Greptile Summary
This PR adds an "Error responses" reference table to the
PasskeyHandler – WebAuthnREADME section, documenting every non-200 status code for all six passkey endpoints. The table is accurate: the 503 row is correctly scoped to the four ceremony endpoints that checkh.WebAuthn == nil, the newBeginRegistrationinvalid-JSON 400 path is present, and theFinishRegistration500 forFindByIDfailures (introduced in #48) is properly captured.Confidence Score: 5/5
Documentation-only change with accurate status codes verified against handler source; safe to merge.
All status codes in the table were cross-checked against handler/passkey.go. The 503 scoping fix and the invalid-JSON 400 addition are correct. Prior review concerns were addressed. No code is modified.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Client Request] --> B{h.WebAuthn == nil?} B -- Yes --> C[503 Service Unavailable\nBeginRegistration / FinishRegistration\nBeginAuthentication / FinishAuthentication only] B -- No --> D{Endpoint} D --> E[BeginRegistration] E --> E1{Valid JSON & name?} E1 -- No --> E2[400 Bad Request] E1 -- Yes --> E3{User/ceremony/store ok?} E3 -- No --> E4[500 Internal Server Error] E3 -- Yes --> E5[200 OK] D --> F[FinishRegistration] F --> F1{session_id param?} F1 -- Missing/expired/wrong user --> F2[400 Bad Request] F1 -- Ok --> F3{WebAuthn verify ok?} F3 -- No --> F4[400 Bad Request] F3 -- Yes --> F5{User lookup & store ok?} F5 -- No --> F6[500 Internal Server Error] F5 -- Yes --> F7[201 Created] D --> G[BeginAuthentication] G --> G1{Ceremony & store ok?} G1 -- No --> G2[500 Internal Server Error] G1 -- Yes --> G3[200 OK] D --> H[FinishAuthentication] H --> H1{session_id param?} H1 -- Missing --> H2[400 Bad Request] H1 -- Ok --> H3{Session valid?} H3 -- No --> H4[401 Unauthorized] H3 -- Yes --> H5{WebAuthn assertion ok?} H5 -- No --> H6[401 Unauthorized] H5 -- Yes --> H7{JWT creation ok?} H7 -- No --> H8[500 Internal Server Error] H7 -- Yes --> H9[200 OK] D --> I[ListCredentials] I --> I1{Store ok?} I1 -- No --> I2[500 Internal Server Error] I1 -- Yes --> I3[200 OK] D --> J[DeleteCredential] J --> J1{credID in URL?} J1 -- Missing --> J2[400 Bad Request] J1 -- Present --> J3{ErrNotFound?} J3 -- Yes --> J4[404 Not Found] J3 -- No/Store error --> J5[500 Internal Server Error] J3 -- Deleted --> J6[204 No Content]Reviews (3): Last reviewed commit: "docs: add credential/user lookup failure..." | Re-trigger Greptile