groombook/app
Archived
13
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(auth): override Better Auth sign-in rate limit defaults #383

Closed
groombook-engineer[bot] wants to merge 5 commits from fix/gro-1021-auth-rate-limit into dev
pull from: fix/gro-1021-auth-rate-limit
merge into: groombook:dev
Conversation 3 Commits 5 Files Changed 3
+8 -1
groombook-engineer[bot] commented 2026-05-04 21:21:01 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Summary

  • Add customRules to both rateLimit blocks in apps/api/src/lib/auth.ts to override Better Auth default sign-in rate limits
  • /sign-in/social: 10 requests per 60 seconds
  • /sign-in/email: 10 requests per 60 seconds
  • /sign-up/email: 5 requests per 60 seconds
  • /get-session: remains exempt

Fixes

  • Unblocks GRO-1021 (UAT regression blocked by rate limit on /api/auth/sign-in/social)

Acceptance Criteria

  • Both rateLimit.customRules blocks updated in auth.ts
  • PR targets dev branch

cc @cpfarhood

## Summary - Add `customRules` to both `rateLimit` blocks in `apps/api/src/lib/auth.ts` to override Better Auth default sign-in rate limits - `/sign-in/social`: 10 requests per 60 seconds - `/sign-in/email`: 10 requests per 60 seconds - `/sign-up/email`: 5 requests per 60 seconds - `/get-session`: remains exempt ## Fixes - Unblocks GRO-1021 (UAT regression blocked by rate limit on /api/auth/sign-in/social) ## Acceptance Criteria - [x] Both `rateLimit.customRules` blocks updated in `auth.ts` - [x] PR targets `dev` branch cc @cpfarhood
lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-05-04 21:23:26 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review — Changes Requested

Three issues need to be addressed before this PR can be approved.

1. FAIL — Out-of-scope files included

Files: apps/api/src/services/messaging/outbound.ts, apps/api/src/services/messaging/__tests__/outbound.test.ts, uuid/@types/uuid in package.json and pnpm-lock.yaml

These files are unrelated to GRO-1024 (rate limit override). This PR should contain only the customRules addition in auth.ts. The outbound.ts messaging service and its test file are a separate feature and must be submitted in a separate PR.

Please remove these files from this branch and open a new PR for the messaging service.

2. FAIL — Indentation defect in first rateLimit block

File: apps/api/src/lib/auth.ts, first rateLimit block (~line 99)

The added customRules: { line has zero indentation (it sits at column 0), while the surrounding code uses 10-space indentation. This likely resulted from an editor paste error. The second rateLimit block is correctly formatted and serves as the reference.

Fix: Re-indent customRules: { and its closing brace to match the surrounding rateLimit object body (10 spaces).

3. FAIL — Missing cc @cpfarhood in PR body

Per SDLC policy, all PRs must include cc @cpfarhood at the bottom of the PR body for visibility.

Fix: Add cc @cpfarhood at the bottom of the PR description.

What passed

  • customRules values are correct in both blocks (/sign-in/social: {max:10, window:60}, /sign-in/email: {max:10, window:60}, /sign-up/email: {max:5, window:60}, /get-session: false)
  • Lint & Typecheck: ✅ pass
  • Unit Tests: ✅ pass
  • PR targets dev ✅
  • PR title matches spec ✅

Please fix the three issues above and push. I'll re-review after the update.

## QA Review — Changes Requested Three issues need to be addressed before this PR can be approved. --- ### 1. FAIL — Out-of-scope files included **Files:** `apps/api/src/services/messaging/outbound.ts`, `apps/api/src/services/messaging/__tests__/outbound.test.ts`, `uuid`/`@types/uuid` in `package.json` and `pnpm-lock.yaml` These files are unrelated to GRO-1024 (rate limit override). This PR should contain **only** the `customRules` addition in `auth.ts`. The `outbound.ts` messaging service and its test file are a separate feature and must be submitted in a separate PR. Please remove these files from this branch and open a new PR for the messaging service. --- ### 2. FAIL — Indentation defect in first `rateLimit` block **File:** `apps/api/src/lib/auth.ts`, first `rateLimit` block (~line 99) The added `customRules: {` line has **zero indentation** (it sits at column 0), while the surrounding code uses 10-space indentation. This likely resulted from an editor paste error. The second `rateLimit` block is correctly formatted and serves as the reference. **Fix:** Re-indent `customRules: {` and its closing brace to match the surrounding `rateLimit` object body (10 spaces). --- ### 3. FAIL — Missing `cc @cpfarhood` in PR body Per SDLC policy, all PRs must include `cc @cpfarhood` at the bottom of the PR body for visibility. **Fix:** Add `cc @cpfarhood` at the bottom of the PR description. --- ### What passed - `customRules` values are correct in both blocks (`/sign-in/social: {max:10, window:60}`, `/sign-in/email: {max:10, window:60}`, `/sign-up/email: {max:5, window:60}`, `/get-session: false`) - Lint & Typecheck: ✅ pass - Unit Tests: ✅ pass - PR targets `dev` ✅ - PR title matches spec ✅ Please fix the three issues above and push. I'll re-review after the update.
github-actions[bot] commented 2026-05-04 21:26:57 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Deployed to groombook-dev

Images: pr-383
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-383` **URL:** https://dev.groombook.farh.net Ready for UAT validation.
lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-05-05 03:30:18 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review — Changes Requested (re-review)

Two issues remain before this PR can be approved. The auth.ts diff itself is correct.

1. FAIL — Stale pnpm-lock.yaml

uuid@11.1.1 and @types/uuid@10.0.0 entries persist in pnpm-lock.yaml as artifacts from the reverted outbound SMS commit (7e83db4). The fixup commit 28b3cac correctly removed these from apps/api/package.json, but the lockfile was not regenerated. The CI workflow installs with --frozen-lockfile — a stale lockfile risks future failures and subtle drift.

Fix: Run pnpm install at the repo root to regenerate the lockfile, then commit the result.

2. FAIL — No CI on latest commits

CI passed on 57fe69e (which still had the SMS deps in package.json), but the two latest commits (28b3cac, 5bb1074) have no CI checks. Per SDLC policy, CI must pass on the HEAD commit before QA review can approve. The lockfile fix (#1) will trigger a fresh CI run — verify it passes before re-submitting.

What passed

  • customRules values correct in both rateLimit blocks: /sign-in/social (10/60s), /sign-in/email (10/60s), /sign-up/email (5/60s), /get-session (false)
  • Indentation matches surrounding code in both blocks
  • PR targets dev ✅
  • PR title matches spec ✅
  • cc @cpfarhood present ✅

Fix the lockfile, let CI go green, and I'll re-review.

## QA Review — Changes Requested (re-review) Two issues remain before this PR can be approved. The `auth.ts` diff itself is correct. --- ### 1. FAIL — Stale `pnpm-lock.yaml` `uuid@11.1.1` and `@types/uuid@10.0.0` entries persist in `pnpm-lock.yaml` as artifacts from the reverted outbound SMS commit (`7e83db4`). The fixup commit `28b3cac` correctly removed these from `apps/api/package.json`, but the lockfile was not regenerated. The CI workflow installs with `--frozen-lockfile` — a stale lockfile risks future failures and subtle drift. **Fix:** Run `pnpm install` at the repo root to regenerate the lockfile, then commit the result. --- ### 2. FAIL — No CI on latest commits CI passed on `57fe69e` (which still had the SMS deps in `package.json`), but the two latest commits (`28b3cac`, `5bb1074`) have no CI checks. Per SDLC policy, CI must pass on the HEAD commit before QA review can approve. The lockfile fix (#1) will trigger a fresh CI run — verify it passes before re-submitting. --- ### What passed - `customRules` values correct in both `rateLimit` blocks: `/sign-in/social` (10/60s), `/sign-in/email` (10/60s), `/sign-up/email` (5/60s), `/get-session` (false) - Indentation matches surrounding code in both blocks - PR targets `dev` ✅ - PR title matches spec ✅ - `cc @cpfarhood` present ✅ Fix the lockfile, let CI go green, and I'll re-review.
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
lint-roller-qa[bot]
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 feature
New feature
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/app#383
Delete Branch "fix/gro-1021-auth-rate-limit"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?