andrii0lomakin commented on code in PR #3137:
URL: https://github.com/apache/tinkerpop/pull/3137#discussion_r2166354019
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java:
##########
@@ -233,15 +236,23 @@ protected void evalOpInternal(final Context ctx, final
Supplier<GremlinExecutor>
final GremlinExecutor.LifeCycle lifeCycle =
GremlinExecutor.LifeCycle.build()
.evaluationTimeoutOverride(seto)
- .afterFailure((b,t) -> {
+ .afterFailure((b, t) -> {
graphManager.onQueryError(msg, t);
- if (managedTransactionsForRequest) attemptRollback(msg,
ctx.getGraphManager(), settings.strictTransactionManagement);
+ if (managedTransactionsForRequest)
+ attemptRollback(msg, ctx.getGraphManager(),
settings.strictTransactionManagement);
+ graphManager.afterQueryEnd(msg);
})
.afterTimeout((b, t) -> {
- graphManager.onQueryError(msg, t);
+ graphManager.onQueryError(msg, t);
+ graphManager.afterQueryEnd(msg);
})
.beforeEval(b -> {
- graphManager.beforeQueryStart(msg);
+ AuthenticatedUser user =
ctx.getChannelHandlerContext().channel().attr(StateKey.AUTHENTICATED_USER).get();
+ if (null == user) {
+ // This is expected when using the
AllowAllAuthenticator
+ user = AuthenticatedUser.ANONYMOUS_USER;
+ }
+ graphManager.beforeQueryStart(msg, user);
Review Comment:
@andrii0lomakin Yes, but that is very high level in what scenario? In a
scenario, when does it already exist in the channel as an attribute? Or should
this test cover all cases of authentication? I believe that such a test case
should either exist or be added as a separate change, as that is not the point
of this PR to test the correctness of authentication mechanics. As that is a
very resource-consuming task, it should be handled separately. I do not mind
providing such testing, as security is paramount for us, but if such tests
already exist in this case, I will only add checking of the parameters of this
callback there. Otherwise, I will test authentication mechanics separately, as
we for sure will not go in prod with authentication holes. Do you have any
objections?
--
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]
