View Issue Details

This bug affects 1 person(s).
 10
IDProjectCategoryView StatusLast Update
09171Bug reportsPluginspublic2014-12-29 15:33
Reporteraesteban Assigned Toaesteban  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version2.05+ 
Summary09171: Unable to subscribe to event
Description

I activate Audit plugin and I subscribe to "afterLogout" event but "afterLogout" funcion from no-plugin code of Limesurvey (./framework/web/auth/CWebUser.php) continue to be called.

Steps To Reproduce

1.- Add a "die" line to existing afterLogout funcion on CWebUser
2.- Activate Audit plugin
3.- Subscribe to afterLogout event.
4.- Implement "alfterLogout" with very simple code (die('I'm in afterLogout');)
5.- Deactivate Audit plugin (in case it could help...) and remove ./tmp/runtime/cache/*
6.- Restart Apache (in case it could help...)
7.- Activate Audit plugin again (in case it could help...)
8.- Login to Limesurvey
9.- Try to logout

Result: You will see message from die of afterLogout of CWebUser.php
Expected result: Message from die of Audit plugin afterLogout function

TagsNo tags attached.
Bug heat10
Complete LimeSurvey version number (& build)140730
I will donate to the project if issue is resolvedNo
BrowserFirefox
Database type & versionPostgresql 9.1.13
Server OS (if known)Debian
Webserver software & version (if known)Apache 2.2.22
PHP VersionPHP 5.4.4

Users monitoring this issue

aesteban

Activities

sammousa

sammousa

2014-11-16 09:14

reporter   ~30990

Hi Alfredo,

The plugin events are separate from yii events.
Limesurveys afterLogout is called in authentication.php on line ~120.

aesteban

aesteban

2014-11-16 18:21

developer   ~30993

Hi Sam,

Thanks for your comment. My problem is that I modify AuditLog.php code but changes are ignored and I think it doesn't depend on where the call to afterLogout is. I paste here the code (as I described above, nothing happens when I call die from afterLogout function either):

diff --git a/plugins/AuditLog/AuditLog.php b/plugins/AuditLog/AuditLog.php
index 501f8e7..bb78dbc 100644
--- a/plugins/AuditLog/AuditLog.php
+++ b/plugins/AuditLog/AuditLog.php
@@ -17,8 +17,19 @@
$this->subscribe('beforePermissionSetSave');
$this->subscribe('beforeParticipantSave');
$this->subscribe('beforeParticipantDelete');

  • $this->subscribe('afterLogout');
    }

  • public function afterLogout()

  • {

  • $oAutoLog = $this->api->newModel($this, 'log');

  • $oAutoLog->uid='uid';

  • $oAutoLog->entity='prueba';

  • $oAutoLog->entityid='666';

  • $oAutoLog->action='action';

  • $oAutoLog->save();

  • }

  •  /**
     * Saves permissions changes to the audit log
     */
aesteban

aesteban

2014-12-06 21:50

developer   ~31188

I found the problem. Function "dispatchEvent" of PluginManager.php accepts an argument "target". This argument is used by Auth plugins and contains the authentication method for current user. If "target" is not empty only the plugin named "target" can process the event:

if (!$event->isStopped()) && (empty($target) || in_array(get_class($subscription[0]), $target)))

This is usefull for authentication plugins. This way, authdb users are managed by authdb plugin and authldap users are managed by authldap plugin.

However, this behaviour implies that no authentication plugins can not use events when target is specified (for example, beforeLogout and afterLogout).

I propose allowing dispatching if plugin isn't an authentication one:

https://github.com/LimeSurvey/LimeSurvey/pull/253

DenisChenu

DenisChenu

2014-12-07 15:28

developer   ~31189

Maybe target must be fixed in autplugin ? Don't know, but i always dislike exception like this.

aesteban

aesteban

2014-12-08 02:53

developer   ~31191

Thanks for your comment, Denis.

Passing "target" as element of _parameters in PluginEvent could be an alternative but we should verify "target" at the beginning of every method in auth plugins to prevent authXXX plugins from processing events of authYYY users. IMHO resulting behaviour is the same than my proposition but writing more code.

What do you think? Any better idea to fix target in auth plugins?

DenisChenu

DenisChenu

2014-12-08 09:09

developer   ~31195

Last edited: 2014-12-08 09:10

I don't have idea, just some thinking.

But looking at this sentence:
"This way, authdb users are managed by authdb plugin and authldap users are managed by authldap plugin." This must be done in the plugin, not by event.

The plugin must look if the target is OK, like we must do for newDirectRequest (to try :add 2 event aaaFirst and bbbSecond, send a die in First without testing target : if you launch plugins/direct?plugin=bbbSecond&function=test the die from first is called.

It's a bug in the plugin, not in LS core.

aesteban

aesteban

2014-12-08 12:51

developer   ~31197

Anyway, something must be done in PluginManager.php. If we don't modify current code, beforeLogout and afterLogout are useless for no auth plugins beyond modifications we could do in plugins.

aesteban

aesteban

2014-12-09 13:37

developer   ~31201

Denis, could you confirm your suggestion is this one?:

  • Remove target param
  • Auth plugin process event only if user has proper auth method
  • Every event is sent to every plugin

If you agree, I will implement this solution.

DenisChenu

DenisChenu

2014-12-09 16:20

developer   ~31220

@aesteban : i don't read the code, only the comment. I think sammoussa have some idea.

aesteban

aesteban

2014-12-18 23:45

developer   ~31302

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=14756

aesteban

aesteban

2014-12-19 00:17

developer   ~31303

Fix committed to 2.06 branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=14757

c_schmitz

c_schmitz

2014-12-29 15:33

administrator   ~31373

Version 2.05 Build 141229 released

Related Changesets

LimeSurvey: master 3984edbf

2014-12-14 21:06:24

aesteban

Details Diff
Fixed issue 09171 : Unable to subscribe to event
Dev : Target not anymore needed for auth plugins events, every auth plugin knows if he has to process event
Dev : This way, auth events (i.e. afterLogout) can be processed also by other plugins
Dev : Method afterLoginFormSubmit can be in parent class (preventing code duplication)
Affected Issues
09171
mod - application/controllers/admin/authentication.php Diff File
mod - application/core/LSUserIdentity.php Diff File
mod - application/core/plugins/AuthLDAP/AuthLDAP.php Diff File
mod - application/core/plugins/Authdb/Authdb.php Diff File
mod - application/core/plugins/Authwebserver/Authwebserver.php Diff File
mod - application/libraries/PluginManager/AuthPluginBase.php Diff File

LimeSurvey: master 978cf724

2014-12-18 22:45:04

aesteban

Details Diff
Merge pull request #260 from Aestu/no_target_for_auth_plugins_4

Fixed issue 09171 : Unable to subscribe to event
Affected Issues
09171
mod - application/controllers/admin/authentication.php Diff File
mod - application/core/LSUserIdentity.php Diff File
mod - application/core/plugins/AuthLDAP/AuthLDAP.php Diff File
mod - application/core/plugins/Authdb/Authdb.php Diff File
mod - application/core/plugins/Authwebserver/Authwebserver.php Diff File
mod - application/libraries/PluginManager/AuthPluginBase.php Diff File

LimeSurvey: 2.06 ec478d08

2014-12-18 23:16:05

aesteban

Details Diff
Fixed issue 09171 : Unable to subscribe to event
Dev : Target not anymore needed for auth plugins events, every auth plugin knows if he has to process event
Dev : This way, auth events (i.e. afterLogout) can be processed also by other plugins
Dev : Method afterLoginFormSubmit can be in parent class (preventing code duplication)
Affected Issues
09171
mod - application/controllers/admin/authentication.php Diff File
mod - application/core/LSUserIdentity.php Diff File
mod - application/core/plugins/AuthLDAP/AuthLDAP.php Diff File
mod - application/core/plugins/Authdb/Authdb.php Diff File
mod - application/core/plugins/Authwebserver/Authwebserver.php Diff File
mod - application/libraries/PluginManager/AuthPluginBase.php Diff File

Issue History

Date Modified Username Field Change
2014-08-14 13:35 aesteban New Issue
2014-08-14 13:37 aesteban Issue Monitored: aesteban
2014-09-11 13:47 c_schmitz Assigned To => mdekker
2014-09-11 13:47 c_schmitz Status new => assigned
2014-11-16 09:14 sammousa Note Added: 30990
2014-11-16 09:14 sammousa Status assigned => feedback
2014-11-16 18:21 aesteban Note Added: 30993
2014-11-16 18:21 aesteban Status feedback => assigned
2014-12-06 21:50 aesteban Note Added: 31188
2014-12-07 15:28 DenisChenu Note Added: 31189
2014-12-08 02:53 aesteban Note Added: 31191
2014-12-08 09:09 DenisChenu Note Added: 31195
2014-12-08 09:10 DenisChenu Note Edited: 31195
2014-12-08 12:51 aesteban Note Added: 31197
2014-12-09 13:37 aesteban Note Added: 31201
2014-12-09 16:20 DenisChenu Note Added: 31220
2014-12-18 23:45 aesteban Changeset attached => LimeSurvey master 3984edbf
2014-12-18 23:45 aesteban Changeset attached => LimeSurvey master 978cf724
2014-12-18 23:45 aesteban Note Added: 31302
2014-12-18 23:45 aesteban Assigned To mdekker => aesteban
2014-12-18 23:45 aesteban Resolution open => fixed
2014-12-18 23:48 aesteban Status assigned => resolved
2014-12-19 00:17 aesteban Changeset attached => LimeSurvey 2.06 ec478d08
2014-12-19 00:17 aesteban Note Added: 31303
2014-12-29 15:33 c_schmitz Note Added: 31373
2014-12-29 15:33 c_schmitz Status resolved => closed
2021-08-04 01:00 guest Bug heat 8 => 10