Skip to content

fix: apply showMemberMiddleware to flowsheet write routes#187

Open
jakebromberg wants to merge 5 commits intomainfrom
fix/10-apply-show-member-middleware
Open

fix: apply showMemberMiddleware to flowsheet write routes#187
jakebromberg wants to merge 5 commits intomainfrom
fix/10-apply-show-member-middleware

Conversation

@jakebromberg
Copy link
Member

Summary

  • Bug: showMemberMiddleware was imported in apps/backend/app.ts but never applied to any route, allowing any authenticated DJ to modify flowsheet entries even without joining the active show.
  • Fix: Applied showMemberMiddleware after requirePermissions on the POST, PATCH, and DELETE flowsheet routes in flowsheet.route.ts. Removed the unused import from app.ts.
  • Hardening: Added try/catch error handling to the middleware (previously had none) and simplified the DJ lookup from .filter().length to .some().

Test plan

  • Unit tests for showMemberMiddleware covering:
    • DJ not in show → returns 400
    • DJ in show → calls next()
    • No DJs in show → returns 400
    • DB error → returns 500 (new error handling)
  • Full unit test suite passes (124 tests, 9 suites)

Fixes #10

Made with Cursor

@jakebromberg jakebromberg force-pushed the fix/10-apply-show-member-middleware branch from 0f349db to f392fe2 Compare February 27, 2026 05:56
Copy link
Collaborator

@AyBruno AyBruno left a comment

Choose a reason for hiding this comment

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

Looks good! This actually wasn't a bug that we weren't using this yet. I wrote it around the time that we were deciding to leave aws cognito and as I has implemented it for our cognito auth specifically, I never hooked it up. Now that we've settled on better-auth, this is a good improvement to clear out some code duplication.

That being said! This PR should include removal of showMember checks within the flowsheet controllers that are preexisting.
Example: https://github.com/WXYC/Backend-Service/blob/main/apps/backend/controllers/flowsheet.controller.ts#L357-L361

Jake Bromberg and others added 3 commits March 9, 2026 22:57
The middleware was imported but never applied, allowing any authenticated
DJ to modify flowsheet entries regardless of show membership.

Co-authored-by: Cursor <cursoragent@cursor.com>
…inline check

Adds showMemberMiddleware to the remaining flowsheet write routes and removes the redundant DJ-in-show check from leaveShow since the middleware now handles it.
@jakebromberg jakebromberg force-pushed the fix/10-apply-show-member-middleware branch from a002084 to 045d898 Compare March 10, 2026 06:29
Jake Bromberg added 2 commits March 9, 2026 23:55
When AUTH_BYPASS=true, requirePermissions skips JWT verification and never populates req.auth. The showMemberMiddleware then fails to identify the user and returns 400 for every request. Adding an AUTH_BYPASS early return fixes integration tests.
The service's guard clause threw a plain Error (500) when a DJ wasn't in the show. With AUTH_BYPASS=true the showMemberMiddleware is skipped, so this code path is reachable during integration tests. Using WxycError returns the proper 400 status.
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.

2 participants