fix(redaction): authorize every ability of a resource type in one request [skip ci]
What does this MR do and why?
Orbit asks Rails to authorize the rows of a query in one redaction request. Some node types share a resource type but need different abilities, for example Project with read_project and Branch with read_code. A Rails client that sets redaction_by_ability now gets one entry for each resource type and ability, in that same single request. Other clients get one entry for each resource type, as before. If Rails leaves out an entry or an ID, the query fails. Before, Orbit dropped the rows with no error.
Related Issues
Relates to #570 and #1313. Next in the stack: !2795 (closed). Rails: gitlab-org/gitlab!261396 (merged) (unknown resource types) and gitlab-org/gitlab!261570 (closed) (sets the flag).
Testing
GDK on gitlab-work-vm-2, with this stack and the Rails change. The user is a Reporter. On project 324 the repository is off, so Rails gives read_project but not read_code.
| Query | Workhorse, no flag | Rails QueryRunner, flag set |
|---|---|---|
| Project 324 | 0 rows, 1 entry | 1 row, 1 entry |
| Project 323 and its branch | 1 row, 1 entry | 1 row, 2 entries |
| Project 324 and its branch | 0 rows, 1 entry | 0 rows, 2 entries |
Every query made one redaction request. The Workhorse column is the same as main.
Performance Analysis
- This merge request does not introduce any performance regression. If a performance regression is expected, explain why.
Every query still makes one redaction request. A client with the flag gets a second entry for a shared resource type, and Rails loads those records once for both abilities.
Agent context: long-form analysis, file-by-file walkthroughs, profiler output, alternatives considered
How it works
-
Flag.
ExecuteQueryRequest.redaction_by_ability(field 5) says that the client answers each entry with its ability. The server copies it to the request context, and the authorization stage reads it. -
Two ability fields.
abilityis the ability that every client can answer. Nodes that share a resource type must use the sameability, and the ontology loader rejects a mismatch.preferred_abilityis optional. With the flag, the authorization stage checks it in place ofability. -
One request.
resource_checksgroups by resource type and ability. All entries go in oneRedactionRequired. -
Matching.
ResourceAuthorization.ability(field 3) tags each answer. Orbit matches an answer on type and ability. An answer with no ability matches the only entry of its type. A missing type, a missing ID, or a repeated answer fails the query.let authorized = answers .remove(&(resource_type.clone(), check.ability.clone())) .or_else(|| answers.remove(&(resource_type.clone(), String::new()))) .filter(|authorized| check.ids.iter().all(|id| authorized.contains_key(id))) .ok_or_else(|| RedactionExchangeError::IncompleteResponse { resource_type: resource_type.clone(), })?;
Compatibility
- Both proto fields are new and optional. Old clients ignore them, and old servers ignore the flag.
- Workhorse and Rails 19.2 to 19.5 never set the flag, so they keep today's behavior.
- An older webserver reads only
ability, so a newer ontology does not change what it sends. On VM-2, main served the v104 ontology from !2795 (closed) on both paths with today's results. - Without the flag, answers must have no ability. With the flag, every answer must have one.
RedactionConfigrejects unknown fields, so a later field cannot be dropped without an error.- The Ruby proto gem needs a release with these fields before the Rails MR can merge.
Other changes
- Deleted
RedactionExchangeError::into_status. Only its own test called it. resource_checksuses aBTreeMap, so the entry order is the same on every run.- The scenario mock reads
resource_type:abilitykeys first, then the type. - The Rails contract check also checks
preferred_ability. Its rule of one ability per resource type moved to the ontology loader. - Scenarios can set
by_abilityto run the flagged path. - Go stubs regenerated with
mise proto:go.