Skip to content

feat(authz): sensible request limit upper bounds #2526

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jul 2, 2025
Merged

feat(authz): sensible request limit upper bounds #2526

merged 10 commits into from
Jul 2, 2025

Conversation

jakedoublev
Copy link
Contributor

Proposed Changes

  • make proto-generate should run toolcheck first and suggest pinned dependency versions for consistency
  • sensible upper bound limits in auth service endpoints
  • protovalidate tests

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

gemini-code-assist[bot]
gemini-code-assist bot reviewed Jul 2, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @jakedoublev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on enhancing the robustness and predictability of the authorization service by implementing sensible upper bounds on various request parameters. These limits are enforced via buf.validate rules in the protobuf definitions, preventing excessively large or malformed requests that could impact service stability or performance. Additionally, the PR improves the development workflow by standardizing protobuf generation tooling.

Highlights

  • Enforced Request Limits: Introduced sensible upper bounds for various fields within the authorization service's protobuf definitions. This includes limiting the number of entities in an EntityIdentifier to 10, attribute values in a Resource to 20, resources in a GetDecisionMultiResourceRequest to 1000, and individual requests in a GetDecisionBulkRequest to 200.
  • Build Process Consistency: Updated the Makefile to pin the version of protoc-gen-connect-openapi to v0.18.0 and ensured that the toolcheck target runs before proto-generate. This helps maintain consistent tooling versions and prevents unexpected build failures due to dependency changes.
  • Documentation Updates: The generated gRPC documentation (docs/grpc/index.html) has been updated to reflect the newly imposed request limits, providing clear information to consumers of the authorization service API.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Limits set with care, Requests now bounded and safe, Systems breathe with ease.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]
gemini-code-assist bot reviewed Jul 2, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces sensible upper bounds for request fields in the authorization service, which is a great step towards making the service more robust. The changes in the Makefile to pin tool versions and ensure tools are present before code generation are also welcome improvements for build consistency. I've added a few suggestions to improve the validation messages and expression efficiency in the protobuf definitions.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 531.709654ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 40.69745ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 349.457987ms
Throughput 286.16 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.764046564s
Average Latency 375.950231ms
Throughput 132.40 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.754926043s
Average Latency 266.681496ms
Throughput 186.88 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 531.333276ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 42.06599ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 348.471139ms
Throughput 286.97 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.322142111s
Average Latency 370.852186ms
Throughput 133.97 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.436201113s
Average Latency 263.541652ms
Throughput 189.13 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 541.752075ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 49.931041ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.710708ms
Throughput 290.94 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.698832814s
Average Latency 365.224865ms
Throughput 136.24 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.966095847s
Average Latency 258.412263ms
Throughput 192.56 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 525.000388ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 37.761951ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 355.007672ms
Throughput 281.68 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.652046623s
Average Latency 374.578031ms
Throughput 132.79 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.338804431s
Average Latency 262.626297ms
Throughput 189.83 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 521.550734ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 39.248248ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 339.268524ms
Throughput 294.75 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.046898564s
Average Latency 358.517649ms
Throughput 138.71 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.519933921s
Average Latency 254.115375ms
Throughput 195.93 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 523.683858ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 40.721737ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 348.696024ms
Throughput 286.78 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.582795089s
Average Latency 374.076097ms
Throughput 133.04 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.202263771s
Average Latency 260.573001ms
Throughput 190.82 requests/second

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 530.871667ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 37.129088ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 347.935811ms
Throughput 287.41 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.615050681s
Average Latency 364.260461ms
Throughput 136.56 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 26.045597736s
Average Latency 259.112363ms
Throughput 191.97 requests/second

@jakedoublev jakedoublev marked this pull request as ready for review July 2, 2025 21:16
@jakedoublev jakedoublev requested review from a team as code owners July 2, 2025 21:16
@jakedoublev jakedoublev enabled auto-merge July 2, 2025 21:16
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 5000
Denied Decision Requests 0
Total Time 531.80102ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Error invalid_argument: validation error:
  • resources: value must contain no more than 1000 item(s) [repeated.max_items] |
    | Total Time | 38.744283ms |

Standard Benchmark Metrics Skipped or Failed

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 341.595226ms
Throughput 292.74 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 36.661768041s
Average Latency 364.570504ms
Throughput 136.38 requests/second

NANOTDF Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 25.8175733s
Average Latency 256.981138ms
Throughput 193.67 requests/second

@jakedoublev jakedoublev added this pull request to the merge queue Jul 2, 2025
Merged via the queue into main with commit b3093cc Jul 2, 2025
32 checks passed
@jakedoublev jakedoublev deleted the feat/DSPX-1261 branch July 2, 2025 22:19
This was referenced Jul 2, 2025
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2025
@opentdf-automation
chore(main): release protocol/go 0.6.0 (#2486)
cf059e9 
🤖 I have created a release *beep* *boop*
---


##
[0.6.0](protocol/go/v0.5.0...protocol/go/v0.6.0)
(2025-07-09)


### Features

* **authz:** sensible request limit upper bounds
([#2526](#2526))
([b3093cc](b3093cc))
* **policy:** Add list key mappings rpc.
([#2533](#2533))
([fbc2724](fbc2724))


### Bug Fixes

* **core:** Allow 521 curve to be used
([#2485](#2485))
([aaf43dc](aaf43dc))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
@opentdf-automation opentdf-automation bot mentioned this pull request Jul 25, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2025
@opentdf-automation
chore(main): release service 0.8.0 (#2487)
b95d4b3 
🤖 I have created a release *beep* *boop*
---


##
[0.8.0](service/v0.7.0...service/v0.8.0)
(2025-07-29)


### Features

* **authz:** RR GetDecision improvements
([#2479](#2479))
([443cedb](443cedb))
* **authz:** sensible request limit upper bounds
([#2526](#2526))
([b3093cc](b3093cc))
* **core:** Add the ability to configure the http server settings
([#2522](#2522))
([b1472df](b1472df))
* **policy:** Add list key mappings rpc.
([#2533](#2533))
([fbc2724](fbc2724))
* **policy:** add obligation protos
([#2579](#2579))
([50882e1](50882e1))
* **policy:** add obligation tables
([#2532](#2532))
([c7d7aa4](c7d7aa4))
* **policy:** Add validation to delete keys
([#2576](#2576))
([cc169d9](cc169d9))
* **policy:** Allow the deletion of a key.
([#2575](#2575))
([82b96f0](82b96f0))
* **policy:** Change return type for delete key proto.
([#2566](#2566))
([c1ae924](c1ae924))
* **policy:** sqlc queries refactor
([#2541](#2541))
([e34680e](e34680e))


### Bug Fixes

* add back grants to listAttributesByDefOrValueFqns
([#2493](#2493))
([2b47095](2b47095))
* **authz:** access pdp should use proto getter
([#2530](#2530))
([f856212](f856212))
* **core:** Allow 521 curve to be used
([#2485](#2485))
([aaf43dc](aaf43dc))
* **core:** resolve 'built-in' typos
([#2548](#2548))
([ccdfa96](ccdfa96))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.2.0 to
0.3.0 in /service
([#2504](#2504))
([a9cc4dd](a9cc4dd))
* **sdk:** Prefer KID and Algorithm selection from key maps
([#2475](#2475))
([98fd392](98fd392))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2025
@opentdf-automation @alkalescent
chore(main): release protocol/go 0.7.0 (#2567)
2c029a3 
🤖 I have created a release *beep* *boop*
---


##
[0.7.0](protocol/go/v0.6.2...protocol/go/v0.7.0)
(2025-08-08)


### ⚠ BREAKING CHANGES

* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))
* **core:** Require go 1.23+
([#1979](#1979))

### Features

* add ability to retrieve policy resources by id or name
([#1901](#1901))
([deb4455](deb4455))
* **authz:** authz v2, ers v2 protos and gencode for ABAC with actions &
registered resource
([#2124](#2124))
([ea7992a](ea7992a))
* **authz:** improve v2 request proto validation
([#2357](#2357))
([f927b99](f927b99))
* **authz:** sensible request limit upper bounds
([#2526](#2526))
([b3093cc](b3093cc))
* **core:** adds bulk rewrap to sdk and service
([#1835](#1835))
([11698ae](11698ae))
* **core:** EXPERIMENTAL: EC-wrapped key support
([#1902](#1902))
([652266f](652266f))
* **core:** Require go 1.23+
([#1979](#1979))
([164c922](164c922))
* **core:** v2 ERS with proto updates
([#2210](#2210))
([a161ef8](a161ef8))
* **policy:** add enhanced standard/custom actions protos
([#2020](#2020))
([bbac53f](bbac53f))
* **policy:** Add legacy keys.
([#2613](#2613))
([57370b0](57370b0))
* **policy:** Add list key mappings rpc.
([#2533](#2533))
([fbc2724](fbc2724))
* **policy:** add obligation protos
([#2579](#2579))
([50882e1](50882e1))
* **policy:** Add validation to delete keys
([#2576](#2576))
([cc169d9](cc169d9))
* **policy:** add values to CreateObligationRequest
([#2614](#2614))
([94535cc](94535cc))
* **policy:** adds new public keys table
([#1836](#1836))
([cad5048](cad5048))
* **policy:** Allow the deletion of a key.
([#2575](#2575))
([82b96f0](82b96f0))
* **policy:** cache SubjectConditionSet selectors in dedicated column
maintained via trigger
([#2320](#2320))
([215791f](215791f))
* **policy:** Change return type for delete key proto.
([#2566](#2566))
([c1ae924](c1ae924))
* **policy:** Default Platform Keys
([#2254](#2254))
([d7447fe](d7447fe))
* **policy:** disable kas grants in favor of key mappings
([#2220](#2220))
([30f8cf5](30f8cf5))
* **policy:** DSPX-1018 NDR retrieval by FQN support
([#2131](#2131))
([0001041](0001041))
* **policy:** DSPX-1057 registered resource action attribute values
(protos only) ([#2217](#2217))
([6375596](6375596))
* **policy:** DSPX-893 NDR define crud protos
([#2056](#2056))
([55a5c27](55a5c27))
* **policy:** DSPX-902 NDR service crud protos only (1/2)
([#2092](#2092))
([24b6cb5](24b6cb5))
* **policy:** Finish resource mapping groups
([#2224](#2224))
([5ff754e](5ff754e))
* **policy:** key management crud
([#2110](#2110))
([4c3d53d](4c3d53d))
* **policy:** Key management proto
([#2115](#2115))
([561f853](561f853))
* **policy:** Modify get request to search for keys by kasid with keyid.
([#2147](#2147))
([780d2e4](780d2e4))
* **policy:** Return KAS Key structure
([#2172](#2172))
([7f97b99](7f97b99))
* **policy:** Return Simple Kas Keys from non-Key RPCs
([#2387](#2387))
([5113e0e](5113e0e))
* **policy:** rotate keys rpc
([#2180](#2180))
([0d00743](0d00743))
* **policy:** Update key status's and UpdateKey rpc.
([#2315](#2315))
([7908db9](7908db9))
* **policy:** Update simple kas key
([#2378](#2378))
([09d8239](09d8239))


### Bug Fixes

* add pagination to list public key mappings response
([#1889](#1889))
([9898fbd](9898fbd))
* **core:** Allow 521 curve to be used
([#2485](#2485))
([aaf43dc](aaf43dc))
* **core:** Fixes protoJSON parse bug on ec rewrap
([#1943](#1943))
([9bebfd0](9bebfd0))
* **core:** Update fixtures and flattening in sdk and service
([#1827](#1827))
([d6d6a7a](d6d6a7a))
* **deps:** bump toolchain in /lib/fixtures and /examples to resolve CVE
GO-2025-3563 ([#2061](#2061))
([9c16843](9c16843))
* **policy:** protovalidate deprecated action types and removal of gRPC
gateway in subject mappings svc
([#2377](#2377))
([54a6de0](54a6de0))
* **policy:** remove gRPC gateway in policy except where needed
([#2382](#2382))
([1937acb](1937acb))
* **policy:** remove new public keys rpc's
([#1962](#1962))
([5049bab](5049bab))
* **policy:** remove predefined rules in actions protos
([#2069](#2069))
([060f059](060f059))
* **policy:** return kas uri on keys for definition, namespace and
values ([#2186](#2186))
([6c55fb8](6c55fb8))
* **sdk:** Fix compatibility between bulk and non-bulk rewrap
([#1914](#1914))
([74abbb6](74abbb6))
* update key_mode to provide more context
([#2226](#2226))
([44d0805](44d0805))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Co-authored-by: Krish Suchak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jentfoo jentfoo jentfoo approved these changes

@elizabethhealy elizabethhealy elizabethhealy approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
comp:authorization docs Documentation size/s
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jakedoublev @jentfoo @elizabethhealy