Add a check if cas client was already created when a new CasManager object is constructed

[parapente]

I run into a problem with using cas()->isAuthenticated() in a middleware where the application works correctly but I could not use pestphp for testing. The reason was that the middleware was evaluated a second time and the execution of CasManager::configureCas would try to create a second phpCAS::client which is not allowed. The result was that phpCAS would exit and as no exception was thrown, pestphp would just fail silently. A simple fix I found was to add a check in CasManager.php::__construct.

[dstepe]

I would like to understand more about the scenario that led to your experience. I'm reluctant to make this change based only an issue executing tests. An attempt to reconfigure the CAS object should be considered an exception. Your change would potentially hide that behavior and make any attempt at diagnosing errors much more difficult.

From your description, it sounds as if executing the middleware twice is the real problem. Working around that in this package would only treat the symptom. Can you share more about the test scenario?

[parapente]

Sure. If you would like to see the code it is in a public repo in github: CCalendar. The problem occurs because in app/Http/Middleware/HandleInertiaRequests.php I am trying to read the user attributes from cas. Before trying to read the attributes, I check cas()->isAuthenticated(). During development and in production, this works fine and the application doesn't break (at least till now). Trying to run the default jetstream/fortify test files using artisan test results in 2 tests being run and then silently exiting. Debugging the execution of pest I found out that for some reason, the middleware is called a second time so cas()->isAuthenticated() is run a second time and that causes the problem as it tries to create a second phpCAS::client.

I don't know if this is the best possible solution for this issue, but I didn't find a way that I could persuade phpCAS to throw an exception instead of just exiting.

[dstepe]

I'm not familiar with jetstream/fortify but if it is similar to other test runners, I believe all tests are run within a single PHP process. The phpCAS package uses private static class members to store it's state and those will persist across multiple tests within the same process. I suspect that's the source of your issue. If you run the tests individually, do they work?

I'm not a fan a few design choices in that project, the static values and hard exit being high on the list. I don't expect we will be successful getting them to change any of that, though.

The best suggestion I can make is to avoid any use of the actual phpCAS package during tests. In your case, you would need to avoid using a static method for getCasUser(). Instead, create an interface which can be injected or resolved in your middleware. In your AppServiceProvider, you can bind different concrete classes depending on if you are running tests or not. When not running tests, you bind a class which uses phpCAS. When running tests, you bind a class that returns whatever data you need for the specific test.

I'd much rather help you with that approach than modify this package in a way that could have unexpected consequences.

[parapente]

If you run the tests individually, do they work?

I haven't tried running a single test from the files which fail to run. I will try it.

The best suggestion I can make is to avoid any use of the actual phpCAS package during tests. In your case, you would need to avoid using a static method for getCasUser(). Instead, create an interface which can be injected or resolved in your middleware. In your AppServiceProvider, you can bind different concrete classes depending on if you are running tests or not. When not running tests, you bind a class which uses phpCAS. When running tests, you bind a class that returns whatever data you need for the specific test.

Thanks for the suggestion! I will try to implement it like that in my application.

I'd much rather help you with that approach than modify this package in a way that could have unexpected consequences.

Understandable.