Add scoped session data scope resolver - #170
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4af7e1202c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (FilterBy filterBy : safeQueryParams(queryParams).toList()) { | ||
| if (queryParamName.equals(filterBy.fieldName)) { | ||
| return CredentialValue.present(filterBy.fieldValue); |
There was a problem hiding this comment.
When the credential source is QUERY_PARAM, this reads the credential but leaves the same FilterBy in the parameters later passed to EffectiveQueryParams. If URL filtering is disabled, an otherwise valid credential can trigger the generic 400 for forbidden filters; if filtering is enabled and the entity has a field with the credential's name, the credential value silently filters the returned data. Treat the credential parameter as transport metadata and exclude it from the parameters processed by the route handler.
Useful? React with 👍 / 👎.
| if (result.dataScopeSelection().isPresent()) { | ||
| final ApiResponse dataScopeResponse = | ||
| dataScopeSelectionApplier.apply(context, result.dataScopeSelection().get()); |
There was a problem hiding this comment.
On routes that also enforce explicit route authentication, this applies the scoped-session selection before RouteAuthPolicy checks the second credential. A valid scoped credential selecting ENSURE_EXISTS or ENSURE_CREATED_AND_POPULATED can therefore persistently create or populate a store even when the required bearer/API-key credential is missing or rejected; defer the mutating selection until every required authentication gate has succeeded.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| @Test | ||
| void authorizerReceivesScopedSessionSelectedScopeAndPrincipal() { |
There was a problem hiding this comment.
This test combines two separable propagation contracts—selected data scope and authenticated principal—so a regression in either concern is reported through one test whose name itself requires “And.” Split them into independently named tests while retaining the shared setup.
AGENTS.md reference: AGENTS.md:L6-L15
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| @Test | ||
| void operationCallbackReceivesScopedSessionPrincipalAndScope() { |
There was a problem hiding this comment.
This callback test verifies two independent context properties in one method, making failures in principal propagation and scope propagation share a single scenario and name. Give each callback contract its own test so the failing behavior is unambiguous.
AGENTS.md reference: AGENTS.md:L6-L15
Useful? React with 👍 / 👎.
Summary
Verification
mvn clean verifymvn install -DskipTestsCloses #168