Repository navigation
Fix batch: reminders, password reset, honeypot, CalDAV, Zoom docs + MCP bearer - #57
Merged
Merged
Conversation
…freshed VerifyMCPBearer's OAuth branch never checked users.archived_at, so archiving a member — the documented offboarding path — left their connected agent bearer working, while sessions and API keys already filtered on it. The refresh grant had the same gap: it would mint fresh tokens for a dead account. Both queries now join users and require archived_at IS NULL, mirroring the API-key fallback directly below. Pinned by TestMCP_OAuthBearerRejectedWhenArchived (red-green verified against the unfixed code on both assertions) and one ARCHITECTURE §6 sentence stating the invariant.
sendReminder joined users on et.user_id, so multi-host teams got reminders naming the event-type creator instead of the booked host. Join on bookings.host_id. loadCancellationData had the same shape, so cancellation/reschedule emails use the booking host too. Pinned by TestWorker_reminderNamesAssignedHost.
The login page linked /admin/forgot-password, which never existed. Add POST /v1/auth/password/forgot (enumeration-safe, background send, skips archived and email-login-disabled accounts) and POST /v1/auth/password/reset (single-use hashed token, 1h TTL, revokes all sessions, signs straight in), mirroring the magic-link flow, plus the two public SPA pages. Pinned by nine tests in password_reset_test.go.
Browsers map a field named company to the organization autofill entry (ignoring autocomplete=off) and fill it for real humans, who are then rejected as bots with 400 invalid submission. Rename to hp_extra on the booking page and embed widget; the stale company key is now an ignored unknown field, so old embeds keep working.
Discovery picked one VEVENT collection and ListCalendars reported only that; conflict checks never read any other collection even when selected. Now discovery returns all collections (seeded with server-side names), ListCalendars lists them live with a stored-selection fallback, and conflict checks REPORT every selected collection. The picker destination (Book) is clearable and the Check/Book headers say what they mean.
An unpublished Zoom app only authorizes same-account users, and Zoom refuses outsiders before Calnode is involved, so member connect failures are policy, not a bug. Document the ranked options (built-in video first) and say so on the settings page, where the old step 2 implied any team member could connect.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
A successful password reset revokes sessions but leaves other unused reset tokens valid, so an earlier unused email can still take over the account.
Reviewed changes
This run reviewed all 25 files across the six commits (MCP archive, reminders, password reset, honeypot, CalDAV multi-calendar, Zoom docs).
- MCP bearer offboarding.
VerifyMCPBearerand refresh now joinusersand refuse archived accounts; the test rotates first so it asserts against the live pair. - Reminder host name.
reminder.sendand cancellationloadCancellationDataload the assignedbookings.host_idinstead of the event-type owner. - Self-service password reset. Hashed single-use tokens, generic 200 on forgot, login-rate-limited routes, and admin pages behind
isPublicRoute. - Honeypot rename. Booking JSON/form/embed field is
hp_extraso browser organization autofill no longer trips the bot check. - CalDAV every collection. Discovery seeds all VEVENT calendars, free/busy REPORTs each selected URL, live
ListCalendarsuses storedcaldav_home_url, and the picker destination control is a clearable checkbox. - Zoom unpublished-app limit. Settings warning plus
docs/ZOOM.mdplaybook.
ℹ️ Nitpicks
Pollalready deletes expired/usedmagic_link_tokensand does not sweep the newpassword_reset_tokenstable, so used and expired reset rows accumulate until someone adds the sibling DELETE.
Grok ｜ 𝕏
| // Unauthenticated request, so there is no current session to preserve — revoke | ||
| // everything (a compromised session must not survive the reset), then sign in. | ||
| if _, err := h.db.ExecContext(r.Context(), | ||
| `DELETE FROM sessions WHERE user_id = ?`, userID); err != nil { |
Contributor
There was a problem hiding this comment.
A successful reset deletes sessions but leaves every other unused password_reset_tokens row valid until expires_at. sendPasswordReset always INSERTs, so three forgot-password clicks yield three live links; using one does not kill the others, and a later unused email can set a new password and revoke the session this handler just created.
Technical details
# Invalidate outstanding reset tokens on success
## Affected sites
- `internal/handler/password_reset.go:161` — session revoke does not touch other reset tokens
- `internal/handler/password_reset.go:73` — each forgot request inserts another live token
## Required outcome
- After a password is actually changed, no previously issued reset token for that user can succeed
- A test that seeds two tokens, consumes one, and asserts the second returns 400
## Suggested approach
After the password UPDATE succeeds, `DELETE FROM password_reset_tokens WHERE user_id = ?` (or set `used_at` on all of them) before creating the new session.# Conflicts: # frontend/src/routes/settings/zoom/+page.svelte
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in this merge. The prior password-reset sibling-token finding is still open.
Reviewed changes
This run reviewed the merge of main (eb04dca) onto d4cb4f2 (commit 11f7271).
- Dropped MCP-archive from this PR. That work already landed as #49, so
VerifyMCPBearer, refresh, and the ARCHITECTURE.md offboarding note no longer appear in this diff. - Resolved Zoom-docs overlap with #52.
docs/ZOOM.mdnow points atDEPLOY.md§8 for Zoom-side distribution instead of duplicating those options. - Kept this PR's settings warning. The Zoom settings page still carries the multi-member banner and now links
VIDEO.mdnext to main's unpublished-app wording.
Grok ｜ 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six commits, all already verified in production on booking.orchestratr.ai:
6f6b31fauth: archived members' MCP bearers stop validating, cannot be refreshed27c5265fix(reminders): name the assigned host, not the event-type owner (Reminder email sent to attendee claims the upcoming meeting is with the wrong person (Solution available). #48, closed)659626dfeat(auth): self-service password reset via emailed link (Forgot password button does not seem to do anything #34, closed, migration 00058)86cc414fix(booking): rename honeypot off the autofill-mapped company name (Error submiting the form to create a new event #33, awaiting reporter confirmation)c22fcc2fix(caldav): discover and check every calendar, not just the first (Connecting to CalDAV only show one calendar #42, awaiting reporter confirmation, migration 00059)d4cb4f2docs(zoom): multi-member playbook and settings-page warning (Issues with zoom integration #35)Closes #48. Closes #34.