kevinoconnor7 / osTicket-auth-cas

JASIG CAS Authentication plugin for osTicket
GNU General Public License v2.0
13 stars 8 forks source link

fix: rename config to cas_config in CasClientAuthBackend and CasStaffAuthBackend #36

Closed larueli closed 1 year ago

larueli commented 1 year ago

Symptom

When updating to osTicket 1.17, several users got the following error : Fatal error: Cannot redeclare non static AuthenticationBackend::$config as static CasStaffAuthBackend::$config in phar://C:/include/plugins/auth-cas.phar/cas.php. See issue #35

Cause

After investigating, it appears that 6 month ago during the development of the multi-instance feature, the following commit was issued to the core osTicket repo : https://github.com/osTicket/osTicket/commit/0b93b48ec6c821e9f182ab2a830ca0adcf24138a. This commit created a new protected attribute config for the AuthenticationBackend class.

However, in the auth-cas plugin, the CasStaffAuthBackend and CasClientAuthBackend both have an already declared config attribute since 7 years, and extends respectively osTicket classes ExternalStaffAuthenticationBackend and ExternalUserAuthenticationBackend, which in turn extends respectively StaffAuthenticationBackend and UserAuthenticationBackend which in turn extends AuthenticationBackend which had the new config attribute. This attribute being only protected, it went all the way down to our classes : CasStaffAuthBackend and CasClientAuthBackend, conflicting with their config attribute.

Solution

Our attribute being private, the easiest solution I have thinked of is changing the name of our variable config to cas_config in our two classes CasStaffAuthBackend and CasClientAuthBackend. Because they are private, they can only be accessed within the source code of theses two classes, so the scope of the fix keeps limited and simple.

Tests

Phar successfully rebuilt and works under osticket v1.17

Additional question

@kevinoconnor7 I can try to provide some fixes or updates as I already done in #33 while being a maintainer of this repo with you if you wish.

I would like to try this things :

That would be already enough for now !

larueli commented 1 year ago

@kevinoconnor7 Maybe you haven't still seen this PR ? Tell me if you need help to understand it or change something !

kevinoconnor7 commented 1 year ago

Sorry… it’s been a hectic week (and next). I promise that this is on my todo list and I’ll get to it as soon as I can!

On Fri, Nov 25, 2022 at 17:20 Ivann LARUELLE @.***> wrote:

@kevinoconnor7 https://github.com/kevinoconnor7 Maybe you haven't still seen this PR ? Tell me if you need help to understand it or change something !

— Reply to this email directly, view it on GitHub https://github.com/kevinoconnor7/osTicket-auth-cas/pull/36#issuecomment-1327913453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAHIMBJXFTHFT4FSDWWM3DWKE3RRANCNFSM6AAAAAASELLYRY . You are receiving this because you were mentioned.Message ID: @.***>

-- Kevin O'Connor kevo.io

larueli commented 1 year ago

No worries, I understand, it's also a side project for me. It was just to make sure that you were still watching this repo ;)

For those who want to build the phar by hand immediatly with the current PR (you should only build locally, not on your production instance for safety reasons -> https://www.php.net/manual/en/phar.configuration.php) :

mkdir test
cd test
git clone https://github.com/larueli/osTicket-auth-cas.git
git clone https://github.com/osTicket/osTicket-plugins.git
rm -rf osTicket-plugins/auth-cas
cp -rf osTicket-auth-cas/auth-cas osTicket-plugins/auth-cas
cd osTicket-plugins
php8.0 -dphar.readonly=0 make.php build auth-cas

You'll find the auth-cas.phar in the current folder, and send it to your server (make sure to make a full backup first).

kevinoconnor7 commented 1 year ago

Cut a new release: https://github.com/kevinoconnor7/osTicket-auth-cas/releases/tag/v1.2.2

Sorry.. took a bit of effort. I update/release so infrequently that something is either broken or some access token expired.

Thank you for your help on this! Your detailed description made this very easy to review and was very much appreciated.

larueli commented 1 year ago

Thanks a lot !

As I said in my first message, if you're looking for a maintainer, I'd like to help you on my free time.

The issue #35 is now solved and can be closed.