I think you have this problem because of the changes from @nicolas-grekas:

if ($container->has('session.factory')) {
        $session = $container->get('session.factory')->createSession();
    } elseif ($container->has('session')) {
        $session = $container->get('session');
    } else {
        return $this;
}

I had also same problem, i was using storage_factory_id: session.storage.factory.mock_file in framework setting and i changed it to storage_id: session.storage.mock_file and now it works.

tbh i'm not sure if his changes is correct, i think we should check if there is already a session before creating a new one right?

So it should be:

if ($container->has('session')) {
        $session = $container->get('session');
    } elseif ($container->has('session.factory')) {
        $session = $container->get('session.factory')->createSession();
    } else {
        return $this;
}

Commit link:
18ab810

I hope this might help you.

So this issue predates these changes by @nicolas-grekas, as 5.3.7 predates his commit. Also once I did update I'm afraid your fix doesn't work. I think it's because I'm only calling loginUser() once but calling request twice? It doesn't matter what happens on subequent calls of loginUser() (though I do think your reccomendation makes sense).

Even if implement your changes to framework.yaml and make this change

    public function testGetChallenges_withInvisible()
    {
        $this->client->loginUser($this->userRepo->findOneBy(['username' => "test_student"]));
        $this->client->request('GET', '/api/classroom/1/challenges');
        $content1 = $this->client->getResponse()->getContent();
        $this->assertEquals(200, $this->client->getResponse()->getStatusCode(), $content1);

        $this->client->loginUser($this->userRepo->findOneBy(['username' => "test_student"]));
        $this->client->request('GET', '/api/classroom/1/challenges?with-invisible=true');
        $content2 = $this->client->getResponse()->getContent();
        $this->assertEquals(200, $this->client->getResponse()->getStatusCode(), $content2);

        $this->assertNotSameSize(json_decode($content1, true), json_decode($content2, true));
    }

It still returns 403.

@Philostastically if you are using a stateless authentication method (which is generally the case for APIs), being authenticated for a request does not make you being authenticated for the next one. You have to send the authentication headers in each request.

@stof Ah, that makes sense. This is my firewall settings for my test env (for the relevant route).

        api:
            pattern: ^/api
            stateless: true
            provider:   our_db_provider

Before I migrated to this version symfony (we were on 5.2), we had a custom Authenticator which had been deprecated because of some security flaws, but we kept it around for the tests because thsoe security flaws made it very easy to write manually in the context of tests. I'd rather not reimplement that authenticator, because it doesn't really represent the state of the server any more. Do you have a reccommendation for what can be done, if loginUser() only works on the first request and not subsequen requests?

Other than recreating the client before each request I guess.

Well, the last example you gave, doing a loginUser before each call, could be fixed. Currently, KernelBrowser reboots the kernel at the beginning of ->request() except for the first one. This is where things fails for now, as the loginUser actions done between both requests are happening on the state of the first request instead of happening after the reboot. I think loginUser() could apply the reboot on its own when needed (and set hasPerformedRequest to false as there would have been no request performed between request).

@stof I have the same issue but not using a stateless authentication method.
I created a reproducer (link in comment linked above)

@adrienfr I know that it also happens with a stateful authentication right now. My previous comment suggests a way to fix it.

tbh i'm not sure if his changes is correct, i think we should check if there is already a session before creating a new one right?

So it should be:

if ($container->has('session')) {
        $session = $container->get('session');
    } elseif ($container->has('session.factory')) {
        $session = $container->get('session.factory')->createSession();
    } else {
        return $this;
}

The change in framework.yaml didn't help but this helps.

The change from the commit broke a test where I'm using the session to store additional information and redirecting after sending a form if there is specific information in the session saved. The redirect after submitting the form now finds an „empty“ session without the needed information and redirects me somewhere else. The change in order of the if-statement fixes this.

@sinansoezen, i was aware that my changes was not correct in the framework.yml any way. the storage_id is deprecated and i had to keep storage_factory_id.

In my case i was using multiple Firewalls in the security.yml and i was also logging in from both of them (one used for normal login, second one is used for API services).

Results: i changed my custom login to login only from one firewall and i add to the API service login settings in security.yml stateless: true as i don't need the login to be saved in session for services. Everything worked as it was before.

@stof I had some free time so I tried implmenting your fix. It worked for the authentication issue, however it did cause one annoying followup issue. First here's my attempted fix:

    /**
     * @param UserInterface $user
     */
    public function loginUser(object $user, string $firewallContext = 'main'): self
    {
        // If we're calling login after the first request, the user will want the
        // the login information to be saved, and if we reboot on the subsequent
        // requests the login that they just did will be undone. So we reboot here
        // instead.
        if ($this->hasPerformedRequest && $this->reboot) {
            $this->kernel->shutdown();
            $this->kernel->boot();
            $this->hasPerformedRequest = false;
        }
       .... The rest of loginUser is the same from here
    }

The issue is that because a User object is retrieved before the reboot of the kernel, the user object will not have equivalence with the same object retreived after the reboot. This is an unrealistic example, but it demonstrates the issue.

    {
        $this->client->loginUser($this->userRepo->findOneBy(['username' => "test_student"]));
        $this->client->request('GET', '/api/classroom/1/challenges');

        $user = $this->userRepo->findOneBy(['username' => "test_student"]);
        $this->client->loginUser($user); 
        $user2 = $this->userRepo->findOneBy(['username' => "test_student"]);
        if( $user == $user2){
            echo "The users are equal";
        } else {
            echo "The users are not equal";
        }
    }

Results in printing The users are not equal. This isn't a big problem in this example, but in some functions we might compare an authenticated user to one retreived by via the objectmanager within the function, and they need to compare correctly if they're the same object.

@stof and @nicolas-grekas do you think we could simply revert conditions such as it was before last change?

if ($container->has('session')) {
    $session = $container->get('session');
} elseif ($container->has('session.factory')) {
    $session = $container->get('session.factory')->createSession();
} else {
    return $this;
}

+1 for a bug.

Additional reproduction repo (without doctrine, tests are performed in-place via github actions)
Repo: https://github.com/Philosoft/symfony-login-as-bug
Test: https://github.com/Philosoft/symfony-login-as-bug/blob/master/tests/functional/MegaTest.php#L39-L56

Fails with symfony/framework-bundle:5.3.8, works as expected with symfony/framework-bundle:5.3.7 (pr: Philosoft/symfony-login-as-bug#1)

General idea - use $client->loginUser() two times in one test and perform two requests.

Additional context

Introduced in this PR #42979 session - works, session.factory - fails

Hi @nicolas-grekas and @stof, I submitted a PR to fix this

I was able to workaround the issue like this

replace the second loginUser:

$client->loginUser($user);

with this:

self::ensureKernelShutdown();
$client = self::createClient()->loginUser($user);

not optimal but that seems to be the smallest change to keep existing tests working

Thank you @guillaumesmo Workaround confirmed Philosoft/symfony-login-as-bug#5

@stof and @nicolas-grekas do you think we could simply revert conditions such as it was before last change?

if ($container->has('session')) {
    $session = $container->get('session');
} elseif ($container->has('session.factory')) {
    $session = $container->get('session.factory')->createSession();
} else {
    return $this;
}

The ordering of this is still a problem I think, if I log a user in and then set a session variable the user is logged out as the session is destroyed. Shouldn't this be corrected so that an existing session is maintained?

Does it still work? I have this problem reproduced. After the first request, the token is reset. I traced where this happens, it is reset to Symfony\Component\HttpKernel\Dependency Injection\ServicesResetter, but I don't understand how it worked for you if the token also had to be reset.

The same issue as @andreymukha