oscerd commented on code in PR #27195:
URL: https://github.com/apache/camel/pull/27195#discussion_r4155809011
##########
components/camel-openfga/src/main/java/org/apache/camel/component/openfga/OpenFgaProducer.java:
##########
@@ -276,6 +292,207 @@ private void listUsers(Exchange exchange,
OpenFgaAuthorizer authorizer) throws E
exchange.getMessage().setBody(identifiers);
}
+ /**
+ * Reads the stored relationship tuples, filtered by whichever of user,
relation and object the endpoint configured.
+ * <p/>
+ * Unlike every other operation these three are optional here: an unset
part means "do not filter on this", and
+ * leaving all three unset reads the whole store a page at a time. That is
why the raw resolvers are used rather
+ * than the check-path ones, which treat a blank value as a missing
identity and deny.
+ */
+ private void readTuples(Exchange exchange, OpenFgaAuthorizer authorizer)
throws Exception {
+ String filterUser = trimmedOrNull(authorizer.rawUser(exchange));
Review Comment:
You are right, and this was the real defect in the set — thank you.
Confirmed exactly as described: `rawUser` evaluates `${header.who}` to
empty, `trimmedOrNull` turns that into `null`, the part is omitted, and because
`object=document:budget` is a legal filter on its own the read comes back with
**every** tuple on the document. Fed into the `readTuples -> deleteTuples`
route the docs recommend, "revoke Bob's access to the budget" becomes "revoke
everyone's".
Fixed in `808b3526ee7a`, along the line you suggested. `OpenFgaAuthorizer`
gained `hasConfiguredUser()` / `hasConfiguredRelation()` /
`hasConfiguredObject()`, which ask the compiled expression rather than the
evaluated value, and `readFilterPart` uses them to keep the two cases apart: an
option the endpoint never set still means "do not filter on this", while a set
option resolving to blank throws and names itself. So it now matches
`hasConfiguredTuple()` on the write path and #27185's deny-on-unresolved, as
you pointed out it should.
**Revert-checked**, because a test passing on fixed code proves nothing:
with `readFilterPart` swapped back for `trimmedOrNull`,
`readTuplesRefusesAFilterPartThatWentMissing` fails with "Expecting actual not
to be null" — no exception was raised at all, the read just went ahead. The
test also asserts `verify(client, never()).read(...)`, so it fails if the
request is ever issued rather than only if the message differs.
This is the read-side form of the fail-open lesson from the original
component review: a value that goes missing must never silently widen scope. I
have written that up in the docs as such rather than as a validation note.
_Claude Code on behalf of oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
