permify icon indicating copy to clipboard operation
permify copied to clipboard

feat(1199): add bulk-permission check wrapper endpoint

Open inabhi9 opened this issue 1 month ago • 4 comments

This PR adds /v1/tenants/{{tenant_id}}/permissions/bulk-check endpoint to check permission in bulk.

It's a simple wrapper around check invoker.

Summary by CodeRabbit

  • New Features

    • Add a bulk permission check API to submit 1–100 authorization checks in a single request with per-item results; individual item failures are returned per-item without cancelling other checks.
  • Documentation

    • API docs and OpenAPI/Swagger specs updated to include the new bulk-check endpoint and request/response schemas.
  • Chores

    • Added request/response validation to enforce field constraints and aggregate validation errors; cancelled requests return an error.

✏️ Tip: You can customize this high-level summary in your review settings.

inabhi9 avatar Nov 26 '25 15:11 inabhi9

Walkthrough

Adds a Permission.BulkCheck RPC, server-side BulkCheck implementation, protobuf messages, generated validation, OpenAPI/Swagger docs, and integration tests that perform batched permission checks.

Changes

Cohort / File(s) Summary
Protobuf service & messages
proto/base/v1/service.proto
Added rpc BulkCheck(PermissionBulkCheckRequest) returns (PermissionBulkCheckResponse) and new messages: PermissionBulkCheckRequestItem, PermissionBulkCheckRequest, PermissionBulkCheckResponse with HTTP mapping POST /v1/tenants/{tenant_id}/permissions/bulk-check.
Server implementation
internal/servers/permission_server.go
Added func (r *PermissionServer) BulkCheck(...): validates tenant_id and item count (1–100), validates items, constructs per-item PermissionCheckRequest, invokes existing Check per item (concurrent coordination via channel/WaitGroup/mutex), maps per-item validation/check errors to DENIED, aggregates and returns PermissionBulkCheckResponse, handles context cancellation.
OpenAPI / Swagger docs
docs/api-reference/apidocs.swagger.json, docs/api-reference/openapiv2/apidocs.swagger.json
Added POST path /v1/tenants/{tenant_id}/permissions/bulk-check, request/response schemas (BulkCheckBody, PermissionBulkCheckRequestItem, PermissionBulkCheckResponse) and operation metadata for the new BulkCheck endpoint.
Generated validation code
pkg/pb/base/v1/service.pb.validate.go
Added validation methods and helpers for new messages (PermissionBulkCheckRequestItem, PermissionBulkCheckRequest, PermissionBulkCheckResponse): Validate/ValidateAll, internal validate(all bool), regex patterns, multi-error wrapper types, and per-field validation error types integrated into the existing validation framework.
Integration tests
integration-test/usecases/facebook_groups_test.go, integration-test/usecases/google_docs_test.go, integration-test/usecases/notion_test.go
Added bulk-check test cases that aggregate existing per-scenario checks into single BulkCheck requests and assert per-item results against expected outcomes.

Sequence Diagram

sequenceDiagram
    autonumber
    actor Client
    participant API as "API Gateway"
    participant Server as "PermissionServer.BulkCheck"
    participant Validator as "Message Validator"
    participant CheckSvc as "Permission.Check"
    participant Aggregator as "Result Aggregator"

    Client->>API: POST /v1/tenants/{tenant_id}/permissions/bulk-check
    API->>Server: BulkCheck(request)
    Server->>Validator: validate tenant_id & items count (1–100)
    alt invalid tenant_id/items
        Server-->>Client: return validation error
    else
        par per-item (concurrent)
            Server->>Validator: validate item
            alt item invalid
                Aggregator-->>Server: append DENIED result for item
            else
                Server->>CheckSvc: Check(per-item PermissionCheckRequest)
                alt check success
                    CheckSvc-->>Aggregator: append item result
                else
                    Aggregator-->>Server: append DENIED result for item
                end
            end
        and continue
        end
        Server-->>Client: PermissionBulkCheckResponse(results[])
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review focus:
    • Mapping per-item validation and Check errors to DENIED in internal/servers/permission_server.go.
    • Concurrency: channel buffering, WaitGroup, mutex correctness, and context cancellation behavior.
    • Generated validation logic in pkg/pb/base/v1/service.pb.validate.go (regexes, multi-error aggregation).
    • Swagger/OpenAPI path and schema consistency.

Poem

🐰 I hopped through batches, checks lined in a row,
For each little subject, I asked "Can they go?"
If one tripped a rule, I scribbled "DENIED" on the slate,
Collected the answers and sent them in a crate.
A bunny’s small audit — swift, tidy, and great.

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 title accurately describes the main change: adding a bulk permission check wrapper endpoint, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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.

coderabbitai[bot] avatar Nov 26 '25 15:11 coderabbitai[bot]

@inabhi9 thank you! can we also add some integration tests for this api. please check /integrations-tests/usecases

ucatbas avatar Dec 08 '25 11:12 ucatbas

@ucatbas sure thing! I wanted some early feedback before adding tests. I see many other endpoints is proxy to engine's functions so not sure if I should move the implementation to engine.

Btw, I found out you also created PR for the same issue and closed. Why it didn't get merge if may I ask?

inabhi9 avatar Dec 09 '25 08:12 inabhi9

@ucatbas sure thing! I wanted some early feedback before adding tests. I see many other endpoints is proxy to engine's functions so not sure if I should move the implementation to engine.

Btw, I found out you also created PR for the same issue and closed. Why it didn't get merge if may I ask?

it was more of a team decision due to bandwidths.

ucatbas avatar Dec 09 '25 17:12 ucatbas

@ucatbas sure thing! I wanted some early feedback before adding tests. I see many other endpoints is proxy to engine's functions so not sure if I should move the implementation to engine.

Btw, I found out you also created PR for the same issue and closed. Why it didn't get merge if may I ask?

for this iteration, i think we can simply remove the network layer for bulk checks as in this PR. moving it into the engine will add more complexity and would require additional validation and optimization. maybe we can revisit that approach later if needed. i will be running some load tests on this version to understand whether that alternative is truly necessary or just over-engineering

ucatbas avatar Dec 12 '25 14:12 ucatbas

Thanks for the PR, @inabhi9. This feature was highly needed, great work!

thisisnkc avatar Dec 15 '25 09:12 thisisnkc