hydrian / TTRSS-Auth-LDAP

GitHub repository for Tiny Tiny RSS's auth_ldap plugin
https://github.com/hydrian/TTRSS-Auth-LDAP/wiki
Other
28 stars 21 forks source link

After changes in auth modules this plugin doesn't work anymore #39

Open young-druid opened 3 years ago

young-druid commented 3 years ago

This commit breaks auth_ldap plugin. I think quite minimal changes are needed in your plugin to work with the latest tt-rss. Current version of plugin gives this error:

Class Auth_Ldap contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (IAuthModule::hook_auth_user)
hydrian commented 3 years ago

Will look into this.

marcpaulchand commented 3 years ago

Hello at all.

I think i fixed it. Here is the patch : L46

- class Auth_Ldap extends Plugin implements IAuthModule {
+ class Auth_Ldap extends Auth_Base {

L76

-         $this->base = new Auth_Base($this->link);

L378

- return $this->base->auto_create_user($ttrssUsername);
+ return $this->auto_create_user($ttrssUsername);

L381

-                    return $this->base->auto_create_user($login);
+                   return $this->auto_create_user($login);

Is it ok for you ?

40 has been created in order to fix it.

But if someone still got a previous version, how to manage both versions ? Like tt-rss is in rolling release, should this repo be always compatible with upstream, and got a branch/tag for previous versions ?

I've been surprised by the auto update which break my installation without notice.

achanu commented 3 years ago

Tested and approved :+1:

Thank you marcpaulchand !

hydrian commented 3 years ago

@marcpaulchand Thanks. I will get it mainlined when I get my dev environment backup.

gramakri commented 3 years ago

The upstream project has moved to using environment variables instead of config.php based 'define'. Maybe we should consider moving to reading configs from env vars as well?

hydrian commented 3 years ago

@gramakri We should look into this but it doesn't belong on this ticket.