Skip to content

Conversation

devkiran
Copy link
Collaborator

@devkiran devkiran commented Oct 17, 2025

Summary by CodeRabbit

  • Refactor
    • Restructured permission validation system for partner profile management endpoints, moving from inline checks to metadata-driven configuration for improved maintainability.

Copy link
Contributor

vercel bot commented Oct 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
dub Ready Ready Preview Oct 17, 2025 8:20am

Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

This PR refactors permission enforcement in partner-profile API routes by centralizing permission checks at the middleware wrapper level. The partnerUser parameter is removed from route handlers, explicit throwIfNoPermission calls are eliminated, and a declarative requiredPermission metadata approach is introduced via the withPartnerProfile wrapper.

Changes

Cohort / File(s) Summary
API Route Refactoring
apps/web/app/(ee)/api/partner-profile/invites/route.ts, apps/web/app/(ee)/api/partner-profile/users/route.ts
Removed partnerUser parameter from POST, PATCH, DELETE handlers; eliminated direct throwIfNoPermission calls; added requiredPermission metadata configurations (e.g., "user_invites.create", "users.update") to each route.
Permission Type Export
apps/web/lib/auth/partner-user-permissions.ts
Exported the Permission type to make it publicly available for external module consumption.
Permission Enforcement Wrapper
apps/web/lib/auth/partner.ts
Extended withPartnerProfile to accept an optional requiredPermission parameter; added WithPartnerProfileOptions interface; integrated runtime permission check via throwIfNoPermission within the wrapper before handler invocation.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Wrapper as withPartnerProfile
    participant Handler as Route Handler
    participant PermCheck as Permission Checker
    participant DB as Database

    Note over Wrapper,Handler: Old Flow (removed)
    Client->>Wrapper: Request
    Wrapper->>Handler: Pass partnerUser
    Handler->>PermCheck: throwIfNoPermission(role, permission)
    alt Permission denied
        PermCheck-->>Handler: Throw error
        Handler-->>Client: 403 Forbidden
    else Permission granted
        Handler->>DB: Process request
        DB-->>Handler: Result
        Handler-->>Client: Response
    end

    Note over Wrapper,Handler: New Flow (added)
    Client->>Wrapper: Request
    Wrapper->>PermCheck: Check requiredPermission?
    alt Permission denied
        PermCheck-->>Wrapper: Throw error
        Wrapper-->>Client: 403 Forbidden
    else Permission granted
        Wrapper->>Handler: Call handler (no partnerUser)
        Handler->>DB: Process request
        DB-->>Handler: Result
        Handler-->>Client: Response
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes follow a consistent refactoring pattern across multiple routes, reducing review complexity. However, the centralized permission enforcement logic in withPartnerProfile introduces new runtime behavior that requires careful verification of permission check placement and error handling integration.

Possibly related PRs

  • dubinc/dub#2958: Modifies the same partner-profile invites/users routes and permission handling mechanisms with overlapping file modifications.

Suggested reviewers

  • steven-tey

Poem

🐰 Permission checks now stand as sentries tall,
No longer scattered through each route's call,
The wrapper whispers: "Have you leave to pass?"
Before the handler sees the looking glass,
Centralized guards make the code so clean! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Move the partner permission checks to withPartnerProfile middleware" accurately describes the main architectural refactoring in this changeset. The core change involves removing explicit throwIfNoPermission calls from three route handlers (POST, PATCH, DELETE in invites and users routes), removing the partnerUser parameter they depended on, and instead centralizing permission enforcement in the withPartnerProfile middleware via a new requiredPermission configuration option. The title is specific and clearly conveys the primary intent: migrating permission checks from inline route logic to a middleware layer. A teammate reviewing pull request history would immediately understand this is a permission-check refactoring focused on the partner profile access layer.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch move-permission-to-middleware

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46a8b5a and e1aaec6.

📒 Files selected for processing (4)
  • apps/web/app/(ee)/api/partner-profile/invites/route.ts (6 hunks)
  • apps/web/app/(ee)/api/partner-profile/users/route.ts (2 hunks)
  • apps/web/lib/auth/partner-user-permissions.ts (1 hunks)
  • apps/web/lib/auth/partner.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/lib/auth/partner.ts (1)
apps/web/lib/auth/partner-user-permissions.ts (2)
  • Permission (4-4)
  • throwIfNoPermission (35-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (14)
apps/web/lib/auth/partner-user-permissions.ts (1)

4-4: LGTM: Permission type now available for external use.

Exporting the Permission type enables the middleware and route handlers to reference permission types consistently.

apps/web/lib/auth/partner.ts (4)

7-7: LGTM: Required imports for permission enforcement.

The imports support the new permission checking functionality in the middleware.


30-32: LGTM: Clean interface for permission options.

The optional requiredPermission field maintains backward compatibility while enabling declarative permission enforcement.


34-37: LGTM: Backward-compatible signature update.

The function signature accepts optional configuration while maintaining backward compatibility with existing routes.


90-95: LGTM: Permission check correctly positioned in middleware flow.

The permission enforcement occurs after authentication and partner retrieval, ensuring the check has the necessary context before the handler executes.

apps/web/app/(ee)/api/partner-profile/users/route.ts (3)

64-64: LGTM: Parameter updated consistently with middleware pattern.

The partnerUser parameter removal is correct since permission enforcement now occurs in the middleware.


130-132: LGTM: Permission check moved to middleware.

The declarative requiredPermission configuration correctly enforces users.update permission at the middleware level.


4-4: Note: Import used only by non-migrated DELETE handler.

The throwIfNoPermission import is currently used only by the DELETE handler, which hasn't been migrated to the middleware pattern yet. Once DELETE is migrated, this import can be removed.

apps/web/app/(ee)/api/partner-profile/invites/route.ts (6)

47-47: LGTM: Parameters updated for middleware-based permissions.

The partnerUser parameter removal aligns with the centralized permission enforcement pattern.


158-160: LGTM: Permission configuration correctly applied.

The user_invites.create permission is properly configured for the POST endpoint.


170-170: LGTM: Parameters updated for middleware-based permissions.

Consistent with the new permission enforcement pattern.


205-207: LGTM: Permission configuration correctly applied.

The user_invites.update permission is properly configured for the PATCH endpoint.


216-216: LGTM: Parameters updated for middleware-based permissions.

Consistent with the new permission enforcement pattern.


238-240: LGTM: Permission configuration correctly applied.

The user_invites.delete permission is properly configured for the DELETE endpoint.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@steven-tey steven-tey merged commit c8cdcba into main Oct 17, 2025
8 of 9 checks passed
@steven-tey steven-tey deleted the move-permission-to-middleware branch October 17, 2025 17:18
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