RequestScoped is destroyed when connection close (disconnect) and another bean is created

brunohpg commented 1 year ago

Describe the bug

Hi,

I'm experiencing strange behavior with RequestScoped. But I'm not sure if it's a bug. In any case, in my view, this could lead to serious security flaws.

When an http request is initiated and the connection is terminated before the response (eg user closes the browser) the RequestScoped beans are destroyed and recreated as if a new request had been initiated.

Switching to SessionScoped the behavior is different: the beans are not destroyed like RequestScoped. However, I discovered this behavior by injecting the SecurityIdentity. In the middle of processing the request, my SecurityIdentitiy becomes anonymous and I lose the request user information. In my case, some methods that write user logs appear as if there is no user.

I believe that SessionScoped is not the solution, as there are other particularities.

Expected behavior

RequestScoped should persist between request->response cycle.

Actual behavior

RequestScoped are destroyed when connection finish and another bean is created.

How to Reproduce?

Simple test:

Create a RequestScoped bean and a Resource:

@RequestScoped
public class RequestScopedBean {
    int value;
   ...Setters and Getters
}

@Path("/")
@ApplicationScoped
public class Resource {

    @Inject
    RequestScopedBean rqb;

    @Path("/test")
    @GET
    protected String test() throws Exception {
        int c = 0;
        while (c++ < 4) {
            rqb.setValue(rqb.getValue()+1);
            Log.infof("Tedt %d == %d", c, rqb.getValue());
            Thread.sleep(10000);
        }
        return "OK";
    }
}

Call /test from an browser and close the page after first log "Test 1 == 1". If the rqb is recreated you will see "Test 2 == 1".

Output of `uname -a` or `ver`

Microsoft Windows [versão 10.0.19045.3086]

Output of `java -version`

OpenJDK Runtime Environment Zulu15.36+13-CA (build 15.0.5+3-MTS)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.7.Final

Build tool (ie. output of `mvnw --version` or `gradlew --version`)

Gradle 7.4.1

Additional information

No response

geoand commented 1 year ago

I was not able to reproduce what you mention.

brunohpg commented 1 year ago

@geoand you are right. You can't reproduce with my example.

Apparently there is another factor that triggers the problem. This is the log I have on my system:

2023-06-23 11:48:18,594 INFO  [br.com.gru.fle.tes.aut.TesteResourceId] (executor-thread-0) 1 Req 1
2023-06-23 11:52:12,903 INFO  [br.com.gru.fle.tes.aut.TesteResourceId] (executor-thread-0) 2 Req 1

I'm trying to find out what's happening. But there's some way to do bug... I'll reply here when I find this factor.

brunohpg commented 1 year ago

Hello. I finally figured out the set of extensions and implementations needed for the bug.

I haven't found a connection between the problem and the necessary extensions and implementations.

For the bug to exist it is necessary:

It is necessary to include io.quarkus:quarkus-reactive-routes or some extension that uses it, like io.quarkus:quarkus-smallrye-jwt;
You need the extensions: io.quarkus:quarkus-security, io.quarkus:quarkus-resteasy and io.quarkus:quarkus-undertow;
You need implement an SecurityIdentityAugmentor;
You need implement an @RouteFilter.

For simplify, I'm attaching one sample: request-scoped-bug.zip

For this sample, call http://localhost:18000/t/test and cancel request in the browser.

The log in console should be:

2023-07-05 11:21:06,911 INFO  [rsb.Resource] (executor-thread-0) Test 1 == 1
2023-07-05 11:21:16,912 INFO  [rsb.Resource] (executor-thread-0) Test 2 == 1

geoand commented 1 year ago

cc @mkouba @sberyozkin

mkouba commented 1 year ago

So from the trace logging I can see that the connection close results in:

2023-07-07 10:57:38,209 TRACE [io.qua.arc.requestContext] (vert.x-eventloop-thread-0) Destroy 7e080c65
        io.quarkus.vertx.web.runtime.RouteHandler$1.handle(RouteHandler.java:93)
        io.quarkus.vertx.web.runtime.RouteHandler$1.handle(RouteHandler.java:90)
        io.vertx.ext.web.impl.RoutingContextImpl.lambda$null$8(RoutingContextImpl.java:532)
        io.vertx.ext.web.impl.SparseArray.forEachInReverseOrder(SparseArray.java:41)
        io.vertx.ext.web.impl.RoutingContextImpl.lambda$getEndHandlers$9(RoutingContextImpl.java:532)
        io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:264)
        io.vertx.core.http.impl.Http1xServerResponse.handleClosed(Http1xServerResponse.java:675)

...which destroys all request scoped beans. And the beans are then recreated.

TBH I have no idea what's the correct behavior and whether it's a security flaw.

brunohpg commented 1 year ago

Hi.

Regarding the behavior, as I said before, I don't know what the correct behavior is either. But I believe that this behavior is very harmful in the processing of requests. The user can manipulate context variables (RequestScoped) during processing, which can lead to unexpected behavior.

About security, the main issue is related to the SecurityIdentity which is RequestScoped. The user is able to "logoff" in the middle of processing and this can bring security issues depending on how the system is implemented. In my case, I check and log the username (principal) several times during processing and I discovered the problem seeing that in the middle of the process the user became anonymous, which for me, is a security issue. I use roles in middle of process which can lead to unexpected behavior.

There's an interesting point: I've been using io.quarkus:quarkus-resteasy, non-reactive version.

mkouba commented 1 year ago

@sberyozkin WDYT?

quarkusio / quarkus