-
-
Notifications
You must be signed in to change notification settings - Fork 276
feat(1199): add bulk-permission check wrapper endpoint #2657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -47,6 +47,82 @@ func (r *PermissionServer) Check(ctx context.Context, request *v1.PermissionChec | |||||||||||||||||||||||||||||||||||||||||||||||||||
| return response, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // BulkCheck - Performs multiple authorization checks in a single request | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (r *PermissionServer) BulkCheck(ctx context.Context, request *v1.PermissionBulkCheckRequest) (*v1.PermissionBulkCheckResponse, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx, span := internal.Tracer.Start(ctx, "permissions.bulk-check") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| defer span.End() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate tenant_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if request.GetTenantId() == "" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := status.Error(GetStatus(nil), "tenant_id is required") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.RecordError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.SetStatus(otelCodes.Error, err.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate number of requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(request.GetItems()) == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := status.Error(GetStatus(nil), "at least one item is required") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.RecordError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.SetStatus(otelCodes.Error, err.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(request.GetItems()) > 100 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := status.Error(GetStatus(nil), "maximum 100 items allowed") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.RecordError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.SetStatus(otelCodes.Error, err.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Process each check request | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| results := make([]*v1.PermissionCheckResponse, len(request.GetItems())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i, checkRequestItem := range request.GetItems() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate individual request | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| v := checkRequestItem.Validate() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if v != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Return error response for this check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| results[i] = &v1.PermissionCheckResponse{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Can: v1.CheckResult_CHECK_RESULT_DENIED, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Metadata: &v1.PermissionCheckResponseMetadata{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| CheckCount: 0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate individual request | |
| v := checkRequestItem.Validate() | |
| if v != nil { | |
| // Return error response for this check | |
| results[i] = &v1.PermissionCheckResponse{ | |
| Can: v1.CheckResult_CHECK_RESULT_DENIED, | |
| Metadata: &v1.PermissionCheckResponseMetadata{ | |
| CheckCount: 0, | |
| }, | |
| } | |
| continue | |
| } | |
| // Validate individual request | |
| v := checkRequestItem.Validate() | |
| if v != nil { | |
| slog.ErrorContext(ctx, "item validation failed in bulk operation", "error", v.Error(), "index", i) | |
| // Return error response for this check | |
| results[i] = &v1.PermissionCheckResponse{ | |
| Can: v1.CheckResult_CHECK_RESULT_DENIED, | |
| Metadata: &v1.PermissionCheckResponseMetadata{ | |
| CheckCount: 0, | |
| }, | |
| } | |
| continue | |
| } |
🤖 Prompt for AI Agents
In internal/servers/permission_server.go around lines 82–93, when a request item
fails validation the code currently returns a DENIED result with no error
details; update handling to (1) add a validation error message into the response
(either by adding an optional ErrorMessage string on PermissionCheckResponse or
adding an Error field on PermissionCheckResponseMetadata) and populate it with
v.Error() (or v.Error() equivalent) for the failing item, and (2) add a
structured server log entry including the item index and the validation error
before continuing so server-side debugging is possible; ensure the proto change
(if adding a field) is backward-compatible/optional and update any
marshaling/use sites accordingly.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting inconsistency.
Line 101 uses tabs for indentation while the surrounding lines use spaces. This creates inconsistent formatting.
Apply this diff to fix the indentation:
checkRequest := &v1.PermissionCheckRequest{
TenantId: request.GetTenantId(),
Subject: checkRequestItem.GetSubject(),
Entity: checkRequestItem.GetEntity(),
Permission: checkRequestItem.GetPermission(),
- Metadata: request.GetMetadata(),
+ Metadata: request.GetMetadata(),
Context: request.GetContext(),
Arguments: request.GetArguments(),
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checkRequest := &v1.PermissionCheckRequest{ | |
| TenantId: request.GetTenantId(), | |
| Subject: checkRequestItem.GetSubject(), | |
| Entity: checkRequestItem.GetEntity(), | |
| Permission: checkRequestItem.GetPermission(), | |
| Metadata: request.GetMetadata(), | |
| Context: request.GetContext(), | |
| Arguments: request.GetArguments(), | |
| } | |
| checkRequest := &v1.PermissionCheckRequest{ | |
| TenantId: request.GetTenantId(), | |
| Subject: checkRequestItem.GetSubject(), | |
| Entity: checkRequestItem.GetEntity(), | |
| Permission: checkRequestItem.GetPermission(), | |
| Metadata: request.GetMetadata(), | |
| Context: request.GetContext(), | |
| Arguments: request.GetArguments(), | |
| } |
🤖 Prompt for AI Agents
In internal/servers/permission_server.go around lines 96 to 104, the indentation
on line 101 uses a tab for the Metadata field unlike the surrounding lines which
use spaces; replace the tab with the same number of spaces used by the other
struct fields so the alignment matches (use the project's existing spacing style
for struct literal fields) and ensure the file passes gofmt/golint formatting
checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @inabhi9, here it is running db checks sequentially, which can slow down bulk operations.
can we process these items concurrently using a worker pool or a similar concurrent aproach?