Development

[dstepe]

Refactor core classes to support testing. Add support for Laravel 11.

[dstepe]

@m0stwan1ed and @ipimpat Thank you both for your willingness to provide updates for Laravel 11 compatibility. The change to add 11 has been merged to the development branch. There are additional changes in development and I could use some feedback before making a new release. If you are able, can you use the dev-development branch in a project and report your experience?

The obvious need for testing is ensuring no regressions under Laravel 11. I have run it a simple app and see no issue.

The more nuanced issue is around logging. When the phpCAS project suddenly deprecated the setDebug() method in favor of setLogger(), they seem to have removed the default log file behavior (or at least I don't see it). Since Laravel uses MonoLog, my initial approach was to simply set that as the logger for phpCAS. While that works, there are two concerns. First, phpCAS logs are very verbose, multi-line, INFO log entries. Second, they may contain data which some users would prefer not to log.

Given those considerations, I am currently redesigning the log configuration to provide more control. I expect the options will be:

1. null (disable logging)
2. laravel (use the Laravel log)
3. file path (create a new log file)
4. custom (provide your own MonoLog instance)

I'd be very interested in other perspectives on this, in addition to testing the changes before release.

[dstepe]

I believe the log options are now in a place I'm willing to move forward with. Please review the new config options (you will need to add this to your existing config/cas.php file or republish it from the package):

    |--------------------------------------------------------------------------
    | Sets the log method for phpCAS. phpCAS logs are verbose, multi-line, INFO
    | log entries. Consider the implications when choosing a log approach.
    |
    | null (default) = no logging
    | laravel = use the Laravel MonoLog instance
    | /path/to/file = create a new log at the given file path
    |--------------------------------------------------------------------------
    */
    'cas_log' => env('CAS_LOG'),

The default behavior is to not use any logger. I believe that is functionally consistent with the previous behavior. The new options allow a simple way to use the Laravel logger or to log to a file of your choice.

It is also possible to inject a log factory implementation and have full control of the instance.

I plan to spend time on the wiki pages next. There are a number of places they are outdated in regard not only to this package but to Laravel current practices. That will not prohibit making a new release soon, but I don't want the docs too far behind. I believe this should be a major release given the internal changes to improve testing and the change in log behaviors.

[dstepe]

The wiki has been updated and some final tweaks added to the development branch. Unless something comes up in testing, I plan to release 5.0.0 in the next couple of days.

[m0stwan1ed]

@dstepe Thanks for your comments and work on an upgrade to Laravel 11.x compatibility.

I've tested the dev-development version on an application upgraded to Laravel 11.x on test and prod environment: all systems OK. We are mostly using cas()->getAttribute('string') method for retrieving attributes from Apereo CAS server. Also, the default cas.auth middleware has been tested - everything works. The only change I've made is cas_log parameter in a config/cas.php file - I've replaced an old CAS_DEBUG field in .env file with new parameter for logging.

I have a question about this part - 'cas_log' => env('CAS_LOG'). Is it ok to leave the default value undefined in this place? I mean, it could not be obvious for everyone, that an empty second field in env() function is null by default. In my opinion, it would be better to pass null as a second argument, as it said in the comment above - null (default) = no logging

[dstepe]

Thanks for help testing. The default value for the second argument of the env() function is null. There is no harm in explicitly including it for clarity, but it also not necessary. It's habit for me to allow defaults in cases like this. However, I can also understand the clarity of explicitly giving the value in the case of a package config file. I've pushed that change.

I believe that all configuration can be done without publishing the cas.php config file now. I plan to begin removing it from our projects. That's one reason I update the wiki with a configuration page listing all the variables.

[dstepe]

@subfission I think you may need to add a review to this PR before I can merge it. I am unable to merge due a review being required and I can't review (probably due to having commits on the branch). I am unable to override the need for a review with my current role as well.

[dstepe]

@subfission I know you are busy, but hoping to get your assistance on this release, or giving me write access so I can complete it. Thanks

[subfission]

Tagging for new minor release