philbertphotos / osticket-multildap-auth-plugin

Plugin for OS Ticket that allows for authentication with multiple domains.
GNU General Public License v3.0
28 stars 16 forks source link

PHP errors showing on latest osTicket #32

Closed EBS-DarkD closed 3 years ago

EBS-DarkD commented 4 years ago

I'm running your plugin on osTicket v1.14.1 and PHP 7.3.14

I've noticed the following 2 errors in the php logs PHP Warning: Declaration of SyncLDAPMultiClass::getconfig($id) should be compatible with LDAPMultiAuthentication::getConfig() in /var/www/html/help/scp/sync_mldap.php on line 0, referer: https://***/scp/logs.php

PHP Warning: Use of undefined constant OST_ROOT - assumed 'OST_ROOT' (this will throw an Error in a future version of PHP) in /var/www/html/help/include/plugins/multi-ldap/config.php on line 316, referer: https://***/scp/logs.php

It seems to be working fine but I thought I should bring this to your attention?

pepper3k commented 4 years ago

yeah that looks important, like an undefined variable, but could also be a config issue, are you sure there's no place to configure that?

not too sure about the first one though

EBS-DarkD commented 4 years ago

I've done a bit of digging on the OST_ROOT, it's part of an AJAX "POST" in function getOptions() Checking my site info page it states to set cgi.fix_pathinfo to "1" if there are any ajax errors. I've tried this and restarted everything but I'm still seeing the issue!

EBS-DarkD commented 4 years ago

Digging a bit further, I see you are defining 'OST_WEB_ROOT' in auth.php. I can't see OST_ROOT defined anywhere?? [edit] edited the config.php to make it OST_WEB_ROOT and the issue has gone out of the logs Still seeing the Declaration error for getconfig()

pepper3k commented 4 years ago

so it looks like changing the variable name solved it. I would, instead of changing the original variable, create a new const as a pointer, so like this:

OST_ROOT='/whatever/ the/real/value/is' OST_WEB_ROOT=OST_ROOT

this way if anything is referencing the old variable name it will still work. just an idea...this way you are basically not meddling with any code of configs, just adding a pointer that will always be updated automatically

because this is a const it will not be changed from within the app, i assume, so it seems safe in my opinion.

regarding the first error, if you paste both SyncLDAPMultiClass::getconfig($id) LDAPMultiAuthentication::getConfig() functions i will take a look

EBS-DarkD commented 4 years ago

My PHP is a bit rusty [auth.php]

define( 'OST_WEB_ROOT', osTicket::get_root_path(--DIR--)); define( 'PLUGINS_ROO', $_SERVER["DOCUMENT_ROOT"] . OST_WEB_ROOT . 'plugins/' );

[config.php] url:"' . OST_ROOT . '/scp/sync_mldap.php?ajax=true",

line changed to url:"' .OST_WEB_ROOT . '/scp/sync_mldap.php?ajax=true",

Will dig into the getconfig functions shortly and paste

philbertphotos commented 4 years ago

I will make an updates. I have a fix for a few things it does work for me without errors ITs just me being lazy and no updating the micro changes I make on a regular.

EBS-DarkD commented 4 years ago

I will make an updates. I have a fix for a few things it does work for me without errors ITs just me being lazy and no updating the micro changes I make on a regular.

I understand perfectly. I'm building clean to change from 1.10 to 1.14 because I was too lazy to do the updates. It's been a bumpy and painful process!

jchrisloyd commented 4 years ago

Did these updates ever get posted? Receiving the following after the OST_ROOT fix.

PHP Warning: Declaration of SyncLDAPMultiClass::getconfig($id) should be compatible with LDAPMultiAuthentication::getConfig() in /var/www/htdocs/osticket/include/scp/sync_mldap.php on line 507

Thanks!

Authentication working, cannot get the sync to work. No errors except for the one above.

philbertphotos commented 3 years ago

I have updated the core so yes its now fixed.