Skip to content

feat: Implement Webhooks Ownership and Authentication Boundary#533

Merged
OlufunbiIK merged 2 commits into
OlufunbiIK:mainfrom
Xaxxoo:feat/webhooks-ownership-auth
Apr 28, 2026
Merged

feat: Implement Webhooks Ownership and Authentication Boundary#533
OlufunbiIK merged 2 commits into
OlufunbiIK:mainfrom
Xaxxoo:feat/webhooks-ownership-auth

Conversation

@Xaxxoo
Copy link
Copy Markdown
Contributor

@Xaxxoo Xaxxoo commented Apr 28, 2026

Fixes #473.

Changes:

  • WebhookPolicyService: Created a new service layer to enforce ownership dynamically on webhooks, stopping users from querying, mutating, or testing webhooks they do not own.
  • JwtAuthGuard: Secured all webhook routes strictly to authenticated requests instead of open API access.
  • DTO Update: Removed strict validation on userId during webhook creation. It's now correctly derived exclusively from the context of the requesting user.
  • Robust Unit Tests: Implemented thorough test suites on ownership, boundary control, and context handling.

Summary by CodeRabbit

  • Security

    • All webhook API endpoints now require JWT authentication.
    • Added ownership validation to ensure users can only access and manage their own webhooks.
  • Improvements

    • User ID is now automatically assigned from authentication context during webhook creation.

Fixes OlufunbiIK#473.
- Created WebhookPolicyService to enforce that a user can only access or mutate their own webhooks.
- Added JwtAuthGuard to WebhooksController to secure all webhook management routes.
- Extracted user identity from the authenticated JWT token (@currentuser) instead of relying on client-provided IDs.
- Updated CreateWebhookDto to make userId optional since it's now inferred securely from the context.
- Wrote extensive unit tests verifying authorization enforcement across all routes.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 28, 2026

@Xaxxoo is attempting to deploy a commit to the olufunbiik's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aefd7906-f4f5-45fb-af7f-c0a5f037da2a

📥 Commits

Reviewing files that changed from the base of the PR and between 8677346 and c6e5b4f.

📒 Files selected for processing (6)
  • backend/src/webhooks/dto/create-webhook.dto.ts
  • backend/src/webhooks/webhook-policy.service.spec.ts
  • backend/src/webhooks/webhook-policy.service.ts
  • backend/src/webhooks/webhooks.controller.spec.ts
  • backend/src/webhooks/webhooks.controller.ts
  • backend/src/webhooks/webhooks.module.ts

📝 Walkthrough

Walkthrough

This pull request implements ownership-based access control for webhook management by making the userId field optional in the DTO (sourced from authenticated user context), introducing a WebhookPolicyService to enforce ownership validation, securing all controller endpoints with JwtAuthGuard, and updating the module configuration to provide the new service.

Changes

Cohort / File(s) Summary
DTO & Configuration
backend/src/webhooks/dto/create-webhook.dto.ts, backend/src/webhooks/webhooks.module.ts
userId field made optional (automatically assigned from auth context); WebhookPolicyService added to module providers and exports.
Access Control Service
backend/src/webhooks/webhook-policy.service.ts, backend/src/webhooks/webhook-policy.service.spec.ts
New WebhookPolicyService introduced with assertOwnership method to validate user ownership of webhooks; test suite verifies ownership validation and ForbiddenException for mismatches.
Controller & Tests
backend/src/webhooks/webhooks.controller.ts, backend/src/webhooks/webhooks.controller.spec.ts
All endpoints now guarded with JwtAuthGuard and use @CurrentUser() for authenticated user context; endpoints that access specific webhooks call policyService.assertOwnership() to enforce access control; controller tests wired with mock user and policy service, updated to verify ownership enforcement.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller as WebhooksController
    participant Guard as JwtAuthGuard
    participant Service as WebhooksService
    participant Policy as WebhookPolicyService
    participant DB as Database

    Client->>Controller: Request (protected endpoint)
    Controller->>Guard: Check JWT token
    Guard-->>Controller: Authenticated user
    
    alt Create Webhook
        Controller->>Service: create(dto, userId from auth)
        Service->>DB: Save webhook with userId
        DB-->>Service: Created webhook
        Service-->>Controller: Return webhook
    else Access Existing Webhook
        Controller->>DB: Load webhook by ID
        DB-->>Controller: Webhook data
        Controller->>Policy: assertOwnership(user, webhook)
        Policy-->>Controller: ✓ Ownership verified
        Controller->>Service: Perform operation
        Service-->>Controller: Return result
    else Unauthorized Access
        Controller->>DB: Load webhook by ID
        DB-->>Controller: Webhook data
        Controller->>Policy: assertOwnership(user, webhook)
        Policy-->>Controller: ✗ ForbiddenException
        Controller-->>Client: 403 Forbidden
    end
    
    Controller-->>Client: Response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Hop, hop, webhooks now with care,
Authentication guards everywhere,
Each user owns what's truly theirs,
No sneaky paws in others' affairs! 🔐

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@OlufunbiIK OlufunbiIK merged commit b356a85 into OlufunbiIK:main Apr 28, 2026
1 of 3 checks passed
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.

Webhooks Ownership and Authentication Boundary

2 participants